diff mbox series

[2/4] phy: phy-can-transceiver: Add support for generic CAN transceiver driver

Message ID 20210409134056.18740-3-a-govindraju@ti.com
State Superseded
Headers show
Series CAN TRANSCEIVER: Add support for CAN transceivers | expand

Commit Message

Aswath Govindraju April 9, 2021, 1:40 p.m. UTC
The driver adds support for generic CAN transceivers. Currently
the modes supported by this driver are standby and normal modes for TI
TCAN1042 and TCAN1043 CAN transceivers.

The transceiver is modelled as a phy with pins controlled by gpios, to put
the transceiver in various device functional modes. It also gets the phy
attribute max_link_rate for the usage of m_can drivers.

Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
---
 drivers/phy/Kconfig               |   9 ++
 drivers/phy/Makefile              |   1 +
 drivers/phy/phy-can-transceiver.c | 140 ++++++++++++++++++++++++++++++
 3 files changed, 150 insertions(+)
 create mode 100644 drivers/phy/phy-can-transceiver.c

Comments

Marc Kleine-Budde April 12, 2021, 10:18 a.m. UTC | #1
On 4/9/21 3:40 PM, Aswath Govindraju wrote:
> The driver adds support for generic CAN transceivers. Currently

> the modes supported by this driver are standby and normal modes for TI

> TCAN1042 and TCAN1043 CAN transceivers.

> 

> The transceiver is modelled as a phy with pins controlled by gpios, to put

> the transceiver in various device functional modes. It also gets the phy

> attribute max_link_rate for the usage of m_can drivers.


This driver should be independent of CAN driver, so you should not mention a
specific driver here.

> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>

> ---

>  drivers/phy/Kconfig               |   9 ++

>  drivers/phy/Makefile              |   1 +

>  drivers/phy/phy-can-transceiver.c | 140 ++++++++++++++++++++++++++++++

>  3 files changed, 150 insertions(+)

>  create mode 100644 drivers/phy/phy-can-transceiver.c

> 

> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig

> index 54c1f2f0985f..51902b629fc6 100644

> --- a/drivers/phy/Kconfig

> +++ b/drivers/phy/Kconfig

> @@ -61,6 +61,15 @@ config USB_LGM_PHY

>  	  interface to interact with USB GEN-II and USB 3.x PHY that is part

>  	  of the Intel network SOC.

>  

> +config PHY_CAN_TRANSCEIVER

> +	tristate "CAN transceiver PHY"

> +	select GENERIC_PHY

> +	help

> +	  This option enables support for CAN transceivers as a PHY. This

> +	  driver provides function for putting the transceivers in various

> +	  functional modes using gpios and sets the attribute max link

> +	  rate, for mcan drivers.

> +

>  source "drivers/phy/allwinner/Kconfig"

>  source "drivers/phy/amlogic/Kconfig"

>  source "drivers/phy/broadcom/Kconfig"

> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile

> index adac1b1a39d1..9c66101c9605 100644

> --- a/drivers/phy/Makefile

> +++ b/drivers/phy/Makefile

> @@ -9,6 +9,7 @@ obj-$(CONFIG_PHY_LPC18XX_USB_OTG)	+= phy-lpc18xx-usb-otg.o

>  obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o

>  obj-$(CONFIG_PHY_PISTACHIO_USB)		+= phy-pistachio-usb.o

>  obj-$(CONFIG_USB_LGM_PHY)		+= phy-lgm-usb.o

> +obj-$(CONFIG_PHY_CAN_TRANSCEIVER)	+= phy-can-transceiver.o

>  obj-y					+= allwinner/	\

>  					   amlogic/	\

>  					   broadcom/	\

> diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c

> new file mode 100644

> index 000000000000..14496f6e1666

> --- /dev/null

> +++ b/drivers/phy/phy-can-transceiver.c

> @@ -0,0 +1,140 @@

> +// SPDX-License-Identifier: GPL-2.0

> +/*

> + * phy-can-transceiver.c - phy driver for CAN transceivers

> + *

> + * Copyright (C) 2021 Texas Instruments Incorporated - http://www.ti.com

> + *

> + */

> +#include<linux/phy/phy.h>

> +#include<linux/platform_device.h>

> +#include<linux/module.h>

> +#include<linux/gpio.h>

> +#include<linux/gpio/consumer.h>

> +

> +struct can_transceiver_data {

> +	u32 flags;

> +#define STB_PRESENT	BIT(0)

> +#define EN_PRESENT	BIT(1)


please add a common prefix to the defines

> +};

> +

> +struct can_transceiver_phy {

> +	struct phy *generic_phy;

> +	struct gpio_desc *standby_gpio;

> +	struct gpio_desc *enable_gpio;

> +};

> +

> +/* Power on function */

> +static int can_transceiver_phy_power_on(struct phy *phy)

> +{

> +	struct can_transceiver_phy *can_transceiver_phy = phy_get_drvdata(phy);

> +

> +	if (can_transceiver_phy->standby_gpio)

> +		gpiod_set_value_cansleep(can_transceiver_phy->standby_gpio, 0);

> +	if (can_transceiver_phy->enable_gpio)

> +		gpiod_set_value_cansleep(can_transceiver_phy->enable_gpio, 1);


Please add a newline before the return.

> +	return 0;

> +}

> +

> +/* Power off function */

> +static int can_transceiver_phy_power_off(struct phy *phy)

> +{

> +	struct can_transceiver_phy *can_transceiver_phy = phy_get_drvdata(phy);

> +

> +	if (can_transceiver_phy->standby_gpio)

> +		gpiod_set_value_cansleep(can_transceiver_phy->standby_gpio, 1);

> +	if (can_transceiver_phy->enable_gpio)

> +		gpiod_set_value_cansleep(can_transceiver_phy->enable_gpio, 0);


same here

> +	return 0;

> +}

> +

> +static const struct phy_ops can_transceiver_phy_ops = {

> +	.power_on	= can_transceiver_phy_power_on,

> +	.power_off	= can_transceiver_phy_power_off,

> +	.owner		= THIS_MODULE,

> +};

> +

> +static const struct can_transceiver_data tcan1042_drvdata = {

> +	.flags = STB_PRESENT,

> +};

> +

> +static const struct can_transceiver_data tcan1043_drvdata = {

> +	.flags = STB_PRESENT | EN_PRESENT,

> +};

> +

> +static const struct of_device_id can_transceiver_phy_ids[] = {

> +	{

> +		.compatible = "ti,tcan1042",

> +		.data = &tcan1042_drvdata

> +	},

> +	{

> +		.compatible = "ti,tcan1043",

> +		.data = &tcan1043_drvdata

> +	},

> +	{ }

> +};

> +MODULE_DEVICE_TABLE(of, can_transceiver_phy_ids);

> +

> +int can_transceiver_phy_probe(struct platform_device *pdev)

> +{

> +	struct phy_provider *phy_provider;

> +	struct device *dev = &pdev->dev;

> +	struct can_transceiver_phy *can_transceiver_phy;

> +	const struct can_transceiver_data *drvdata;

> +	const struct of_device_id *match;

> +	struct phy *phy;

> +	struct gpio_desc *standby_gpio;

> +	struct gpio_desc *enable_gpio;

> +	u32 max_bitrate = 0;

> +

> +	can_transceiver_phy = devm_kzalloc(dev, sizeof(struct can_transceiver_phy), GFP_KERNEL);


error handling?

> +

> +	match = of_match_node(can_transceiver_phy_ids, pdev->dev.of_node);

> +	drvdata = match->data;

> +

> +	phy = devm_phy_create(dev, dev->of_node,

> +			      &can_transceiver_phy_ops);

> +	if (IS_ERR(phy)) {

> +		dev_err(dev, "failed to create can transceiver phy\n");

> +		return PTR_ERR(phy);

> +	}

> +

> +	device_property_read_u32(dev, "max-bitrate", &max_bitrate);

> +	phy->attrs.max_link_rate = max_bitrate / 1000000;


The problem is, there are CAN transceivers with a max of 83.3 kbit/s or 125 kbit/s.

> +	can_transceiver_phy->generic_phy = phy;

> +

> +	if (drvdata->flags & STB_PRESENT) {

> +		standby_gpio = devm_gpiod_get(dev, "standby",   GPIOD_OUT_LOW);


please use only one space after the ",".
Why do you request the gpio standby low?

> +		if (IS_ERR(standby_gpio))

> +			return PTR_ERR(standby_gpio);

> +		can_transceiver_phy->standby_gpio = standby_gpio;

> +	}

> +

> +	if (drvdata->flags & EN_PRESENT) {

> +		enable_gpio = devm_gpiod_get(dev, "enable",   GPIOD_OUT_LOW);

> +		if (IS_ERR(enable_gpio))

> +			return PTR_ERR(enable_gpio);

> +		can_transceiver_phy->enable_gpio = enable_gpio;

> +	}

> +

> +	phy_set_drvdata(can_transceiver_phy->generic_phy, can_transceiver_phy);

> +

> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);

> +

> +	return PTR_ERR_OR_ZERO(phy_provider);

> +}

> +

> +static struct platform_driver can_transceiver_phy_driver = {

> +	.probe = can_transceiver_phy_probe,

> +	.driver = {

> +		.name = "can-transceiver-phy",

> +		.of_match_table = can_transceiver_phy_ids,

> +	},

> +};

> +

> +module_platform_driver(can_transceiver_phy_driver);

> +

> +MODULE_AUTHOR("Faiz Abbas <faiz_abbas@ti.com>");

> +MODULE_AUTHOR("Aswath Govindraju <a-govindraju@ti.com>");

> +MODULE_DESCRIPTION("CAN TRANSCEIVER PHY driver");

> +MODULE_LICENSE("GPL v2");

> 


marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |
Aswath Govindraju April 14, 2021, 6:24 a.m. UTC | #2
Hi Marc,

On 12/04/21 3:48 pm, Marc Kleine-Budde wrote:
> On 4/9/21 3:40 PM, Aswath Govindraju wrote:

>> The driver adds support for generic CAN transceivers. Currently

>> the modes supported by this driver are standby and normal modes for TI

>> TCAN1042 and TCAN1043 CAN transceivers.

>>

>> The transceiver is modelled as a phy with pins controlled by gpios, to put

>> the transceiver in various device functional modes. It also gets the phy

>> attribute max_link_rate for the usage of m_can drivers.

> 

> This driver should be independent of CAN driver, so you should not mention a

> specific driver here.

> 


I will substitute m_can with can in the respin.

>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>

>> ---

>>  drivers/phy/Kconfig               |   9 ++

>>  drivers/phy/Makefile              |   1 +

>>  drivers/phy/phy-can-transceiver.c | 140 ++++++++++++++++++++++++++++++

>>  3 files changed, 150 insertions(+)

>>  create mode 100644 drivers/phy/phy-can-transceiver.c

>>

>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig

>> index 54c1f2f0985f..51902b629fc6 100644

>> --- a/drivers/phy/Kconfig

>> +++ b/drivers/phy/Kconfig

>> @@ -61,6 +61,15 @@ config USB_LGM_PHY

>>  	  interface to interact with USB GEN-II and USB 3.x PHY that is part

>>  	  of the Intel network SOC.

>>  

>> +config PHY_CAN_TRANSCEIVER

>> +	tristate "CAN transceiver PHY"

>> +	select GENERIC_PHY

>> +	help

>> +	  This option enables support for CAN transceivers as a PHY. This

>> +	  driver provides function for putting the transceivers in various

>> +	  functional modes using gpios and sets the attribute max link

>> +	  rate, for mcan drivers.

>> +

>>  source "drivers/phy/allwinner/Kconfig"

>>  source "drivers/phy/amlogic/Kconfig"

>>  source "drivers/phy/broadcom/Kconfig"

>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile

>> index adac1b1a39d1..9c66101c9605 100644

>> --- a/drivers/phy/Makefile

>> +++ b/drivers/phy/Makefile

>> @@ -9,6 +9,7 @@ obj-$(CONFIG_PHY_LPC18XX_USB_OTG)	+= phy-lpc18xx-usb-otg.o

>>  obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o

>>  obj-$(CONFIG_PHY_PISTACHIO_USB)		+= phy-pistachio-usb.o

>>  obj-$(CONFIG_USB_LGM_PHY)		+= phy-lgm-usb.o

>> +obj-$(CONFIG_PHY_CAN_TRANSCEIVER)	+= phy-can-transceiver.o

>>  obj-y					+= allwinner/	\

>>  					   amlogic/	\

>>  					   broadcom/	\

>> diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c

>> new file mode 100644

>> index 000000000000..14496f6e1666

>> --- /dev/null

>> +++ b/drivers/phy/phy-can-transceiver.c

>> @@ -0,0 +1,140 @@

>> +// SPDX-License-Identifier: GPL-2.0

>> +/*

>> + * phy-can-transceiver.c - phy driver for CAN transceivers

>> + *

>> + * Copyright (C) 2021 Texas Instruments Incorporated - http://www.ti.com

>> + *

>> + */

>> +#include<linux/phy/phy.h>

>> +#include<linux/platform_device.h>

>> +#include<linux/module.h>

>> +#include<linux/gpio.h>

>> +#include<linux/gpio/consumer.h>

>> +

>> +struct can_transceiver_data {

>> +	u32 flags;

>> +#define STB_PRESENT	BIT(0)

>> +#define EN_PRESENT	BIT(1)

> 

> please add a common prefix to the defines


I will add a common prefix(GPIO) in the respin.

> 

>> +};

>> +

>> +struct can_transceiver_phy {

>> +	struct phy *generic_phy;

>> +	struct gpio_desc *standby_gpio;

>> +	struct gpio_desc *enable_gpio;

>> +};

>> +

>> +/* Power on function */

>> +static int can_transceiver_phy_power_on(struct phy *phy)

>> +{

>> +	struct can_transceiver_phy *can_transceiver_phy = phy_get_drvdata(phy);

>> +

>> +	if (can_transceiver_phy->standby_gpio)

>> +		gpiod_set_value_cansleep(can_transceiver_phy->standby_gpio, 0);

>> +	if (can_transceiver_phy->enable_gpio)

>> +		gpiod_set_value_cansleep(can_transceiver_phy->enable_gpio, 1);

> 

> Please add a newline before the return.

> 


Will make this change in the respin.

>> +	return 0;

>> +}

>> +

>> +/* Power off function */

>> +static int can_transceiver_phy_power_off(struct phy *phy)

>> +{

>> +	struct can_transceiver_phy *can_transceiver_phy = phy_get_drvdata(phy);

>> +

>> +	if (can_transceiver_phy->standby_gpio)

>> +		gpiod_set_value_cansleep(can_transceiver_phy->standby_gpio, 1);

>> +	if (can_transceiver_phy->enable_gpio)

>> +		gpiod_set_value_cansleep(can_transceiver_phy->enable_gpio, 0);

> 

> same here

> 


Will make this change in the respin

>> +	return 0;

>> +}

>> +

>> +static const struct phy_ops can_transceiver_phy_ops = {

>> +	.power_on	= can_transceiver_phy_power_on,

>> +	.power_off	= can_transceiver_phy_power_off,

>> +	.owner		= THIS_MODULE,

>> +};

>> +

>> +static const struct can_transceiver_data tcan1042_drvdata = {

>> +	.flags = STB_PRESENT,

>> +};

>> +

>> +static const struct can_transceiver_data tcan1043_drvdata = {

>> +	.flags = STB_PRESENT | EN_PRESENT,

>> +};

>> +

>> +static const struct of_device_id can_transceiver_phy_ids[] = {

>> +	{

>> +		.compatible = "ti,tcan1042",

>> +		.data = &tcan1042_drvdata

>> +	},

>> +	{

>> +		.compatible = "ti,tcan1043",

>> +		.data = &tcan1043_drvdata

>> +	},

>> +	{ }

>> +};

>> +MODULE_DEVICE_TABLE(of, can_transceiver_phy_ids);

>> +

>> +int can_transceiver_phy_probe(struct platform_device *pdev)

>> +{

>> +	struct phy_provider *phy_provider;

>> +	struct device *dev = &pdev->dev;

>> +	struct can_transceiver_phy *can_transceiver_phy;

>> +	const struct can_transceiver_data *drvdata;

>> +	const struct of_device_id *match;

>> +	struct phy *phy;

>> +	struct gpio_desc *standby_gpio;

>> +	struct gpio_desc *enable_gpio;

>> +	u32 max_bitrate = 0;

>> +

>> +	can_transceiver_phy = devm_kzalloc(dev, sizeof(struct can_transceiver_phy), GFP_KERNEL);

> 

> error handling?

> 


Will add this in the respin.

>> +

>> +	match = of_match_node(can_transceiver_phy_ids, pdev->dev.of_node);

>> +	drvdata = match->data;

>> +

>> +	phy = devm_phy_create(dev, dev->of_node,

>> +			      &can_transceiver_phy_ops);

>> +	if (IS_ERR(phy)) {

>> +		dev_err(dev, "failed to create can transceiver phy\n");

>> +		return PTR_ERR(phy);

>> +	}

>> +

>> +	device_property_read_u32(dev, "max-bitrate", &max_bitrate);

>> +	phy->attrs.max_link_rate = max_bitrate / 1000000;

> 

> The problem is, there are CAN transceivers with a max of 83.3 kbit/s or 125 kbit/s.

> 


The only way that I was able to find for this is to add a phy attribute
"max_bit_rate" in include/linux/phy/phy.h. Would this be an acceptable
solution ?

>> +	can_transceiver_phy->generic_phy = phy;

>> +

>> +	if (drvdata->flags & STB_PRESENT) {

>> +		standby_gpio = devm_gpiod_get(dev, "standby",   GPIOD_OUT_LOW);

> 

> please use only one space after the ",".


Will correct this in respin.

> Why do you request the gpio standby low?


While probing the transceiver has to be in standby state and only after
calling the power on does the transceiver go to enable state. This was
the reason behind requesting gpio standby low.

Thank you for for review.

Regards,
Aswath

> 

>> +		if (IS_ERR(standby_gpio))

>> +			return PTR_ERR(standby_gpio);

>> +		can_transceiver_phy->standby_gpio = standby_gpio;

>> +	}

>> +

>> +	if (drvdata->flags & EN_PRESENT) {

>> +		enable_gpio = devm_gpiod_get(dev, "enable",   GPIOD_OUT_LOW);

>> +		if (IS_ERR(enable_gpio))

>> +			return PTR_ERR(enable_gpio);

>> +		can_transceiver_phy->enable_gpio = enable_gpio;

>> +	}

>> +

>> +	phy_set_drvdata(can_transceiver_phy->generic_phy, can_transceiver_phy);

>> +

>> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);

>> +

>> +	return PTR_ERR_OR_ZERO(phy_provider);

>> +}

>> +

>> +static struct platform_driver can_transceiver_phy_driver = {

>> +	.probe = can_transceiver_phy_probe,

>> +	.driver = {

>> +		.name = "can-transceiver-phy",

>> +		.of_match_table = can_transceiver_phy_ids,

>> +	},

>> +};

>> +

>> +module_platform_driver(can_transceiver_phy_driver);

>> +

>> +MODULE_AUTHOR("Faiz Abbas <faiz_abbas@ti.com>");

>> +MODULE_AUTHOR("Aswath Govindraju <a-govindraju@ti.com>");

>> +MODULE_DESCRIPTION("CAN TRANSCEIVER PHY driver");

>> +MODULE_LICENSE("GPL v2");

>>

> 

> marc

>
Marc Kleine-Budde April 14, 2021, 6:57 a.m. UTC | #3
On 14.04.2021 11:54:36, Aswath Govindraju wrote:
> Hi Marc,

> 

> On 12/04/21 3:48 pm, Marc Kleine-Budde wrote:

> > On 4/9/21 3:40 PM, Aswath Govindraju wrote:

> >> The driver adds support for generic CAN transceivers. Currently

> >> the modes supported by this driver are standby and normal modes for TI

> >> TCAN1042 and TCAN1043 CAN transceivers.

> >>

> >> The transceiver is modelled as a phy with pins controlled by gpios, to put

> >> the transceiver in various device functional modes. It also gets the phy

> >> attribute max_link_rate for the usage of m_can drivers.

> > 

> > This driver should be independent of CAN driver, so you should not mention a

> > specific driver here.

> > 

> 

> I will substitute m_can with can in the respin.


Better use uppercase CAN instead of can.

[...]

> >> diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c

> >> new file mode 100644

> >> index 000000000000..14496f6e1666

> >> --- /dev/null

> >> +++ b/drivers/phy/phy-can-transceiver.c

> >> @@ -0,0 +1,140 @@

> >> +// SPDX-License-Identifier: GPL-2.0

> >> +/*

> >> + * phy-can-transceiver.c - phy driver for CAN transceivers

> >> + *

> >> + * Copyright (C) 2021 Texas Instruments Incorporated - http://www.ti.com

> >> + *

> >> + */

> >> +#include<linux/phy/phy.h>

> >> +#include<linux/platform_device.h>

> >> +#include<linux/module.h>

> >> +#include<linux/gpio.h>

> >> +#include<linux/gpio/consumer.h>

> >> +

> >> +struct can_transceiver_data {

> >> +	u32 flags;

> >> +#define STB_PRESENT	BIT(0)

> >> +#define EN_PRESENT	BIT(1)

> > 

> > please add a common prefix to the defines

> 

> I will add a common prefix(GPIO) in the respin.


I was thinking about something like CAN_TRANSCEIVER_

[...]

> >> +int can_transceiver_phy_probe(struct platform_device *pdev)

> >> +{

> >> +	struct phy_provider *phy_provider;

> >> +	struct device *dev = &pdev->dev;

> >> +	struct can_transceiver_phy *can_transceiver_phy;

> >> +	const struct can_transceiver_data *drvdata;

> >> +	const struct of_device_id *match;

> >> +	struct phy *phy;

> >> +	struct gpio_desc *standby_gpio;

> >> +	struct gpio_desc *enable_gpio;

> >> +	u32 max_bitrate = 0;

> >> +

> >> +	can_transceiver_phy = devm_kzalloc(dev, sizeof(struct can_transceiver_phy), GFP_KERNEL);

> > 

> > error handling?

> > 

> 

> Will add this in the respin.

> 

> >> +

> >> +	match = of_match_node(can_transceiver_phy_ids, pdev->dev.of_node);

> >> +	drvdata = match->data;

> >> +

> >> +	phy = devm_phy_create(dev, dev->of_node,

> >> +			      &can_transceiver_phy_ops);

> >> +	if (IS_ERR(phy)) {

> >> +		dev_err(dev, "failed to create can transceiver phy\n");

> >> +		return PTR_ERR(phy);

> >> +	}

> >> +

> >> +	device_property_read_u32(dev, "max-bitrate", &max_bitrate);

> >> +	phy->attrs.max_link_rate = max_bitrate / 1000000;

> > 

> > The problem is, there are CAN transceivers with a max of 83.3 kbit/s or 125 kbit/s.

> > 

> 

> The only way that I was able to find for this is to add a phy attribute

> "max_bit_rate" in include/linux/phy/phy.h. Would this be an acceptable

> solution ?


I think that's up to the phy people.

Another solution would be to have a public struct can_transceiver:

| struct can_transceiver { 
| 	struct phy *generic_phy;
|       u32 max_bitrate;
| };

which holds the max_bitrate. In the CAN controller driver you can use
container_of to get that struct and access the max_bitrate.

> >> +	can_transceiver_phy->generic_phy = phy;

> >> +

> >> +	if (drvdata->flags & STB_PRESENT) {

> >> +		standby_gpio = devm_gpiod_get(dev, "standby",   GPIOD_OUT_LOW);

> > 

> > please use only one space after the ",".

> 

> Will correct this in respin.

> 

> > Why do you request the gpio standby low?

> 

> While probing the transceiver has to be in standby state and only after

> calling the power on does the transceiver go to enable state. This was

> the reason behind requesting gpio standby low.


This isn't consistent with the power_on and power_off functions.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |
diff mbox series

Patch

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 54c1f2f0985f..51902b629fc6 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -61,6 +61,15 @@  config USB_LGM_PHY
 	  interface to interact with USB GEN-II and USB 3.x PHY that is part
 	  of the Intel network SOC.
 
+config PHY_CAN_TRANSCEIVER
+	tristate "CAN transceiver PHY"
+	select GENERIC_PHY
+	help
+	  This option enables support for CAN transceivers as a PHY. This
+	  driver provides function for putting the transceivers in various
+	  functional modes using gpios and sets the attribute max link
+	  rate, for mcan drivers.
+
 source "drivers/phy/allwinner/Kconfig"
 source "drivers/phy/amlogic/Kconfig"
 source "drivers/phy/broadcom/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index adac1b1a39d1..9c66101c9605 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -9,6 +9,7 @@  obj-$(CONFIG_PHY_LPC18XX_USB_OTG)	+= phy-lpc18xx-usb-otg.o
 obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
 obj-$(CONFIG_PHY_PISTACHIO_USB)		+= phy-pistachio-usb.o
 obj-$(CONFIG_USB_LGM_PHY)		+= phy-lgm-usb.o
+obj-$(CONFIG_PHY_CAN_TRANSCEIVER)	+= phy-can-transceiver.o
 obj-y					+= allwinner/	\
 					   amlogic/	\
 					   broadcom/	\
diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c
new file mode 100644
index 000000000000..14496f6e1666
--- /dev/null
+++ b/drivers/phy/phy-can-transceiver.c
@@ -0,0 +1,140 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * phy-can-transceiver.c - phy driver for CAN transceivers
+ *
+ * Copyright (C) 2021 Texas Instruments Incorporated - http://www.ti.com
+ *
+ */
+#include<linux/phy/phy.h>
+#include<linux/platform_device.h>
+#include<linux/module.h>
+#include<linux/gpio.h>
+#include<linux/gpio/consumer.h>
+
+struct can_transceiver_data {
+	u32 flags;
+#define STB_PRESENT	BIT(0)
+#define EN_PRESENT	BIT(1)
+};
+
+struct can_transceiver_phy {
+	struct phy *generic_phy;
+	struct gpio_desc *standby_gpio;
+	struct gpio_desc *enable_gpio;
+};
+
+/* Power on function */
+static int can_transceiver_phy_power_on(struct phy *phy)
+{
+	struct can_transceiver_phy *can_transceiver_phy = phy_get_drvdata(phy);
+
+	if (can_transceiver_phy->standby_gpio)
+		gpiod_set_value_cansleep(can_transceiver_phy->standby_gpio, 0);
+	if (can_transceiver_phy->enable_gpio)
+		gpiod_set_value_cansleep(can_transceiver_phy->enable_gpio, 1);
+	return 0;
+}
+
+/* Power off function */
+static int can_transceiver_phy_power_off(struct phy *phy)
+{
+	struct can_transceiver_phy *can_transceiver_phy = phy_get_drvdata(phy);
+
+	if (can_transceiver_phy->standby_gpio)
+		gpiod_set_value_cansleep(can_transceiver_phy->standby_gpio, 1);
+	if (can_transceiver_phy->enable_gpio)
+		gpiod_set_value_cansleep(can_transceiver_phy->enable_gpio, 0);
+	return 0;
+}
+
+static const struct phy_ops can_transceiver_phy_ops = {
+	.power_on	= can_transceiver_phy_power_on,
+	.power_off	= can_transceiver_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static const struct can_transceiver_data tcan1042_drvdata = {
+	.flags = STB_PRESENT,
+};
+
+static const struct can_transceiver_data tcan1043_drvdata = {
+	.flags = STB_PRESENT | EN_PRESENT,
+};
+
+static const struct of_device_id can_transceiver_phy_ids[] = {
+	{
+		.compatible = "ti,tcan1042",
+		.data = &tcan1042_drvdata
+	},
+	{
+		.compatible = "ti,tcan1043",
+		.data = &tcan1043_drvdata
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, can_transceiver_phy_ids);
+
+int can_transceiver_phy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *phy_provider;
+	struct device *dev = &pdev->dev;
+	struct can_transceiver_phy *can_transceiver_phy;
+	const struct can_transceiver_data *drvdata;
+	const struct of_device_id *match;
+	struct phy *phy;
+	struct gpio_desc *standby_gpio;
+	struct gpio_desc *enable_gpio;
+	u32 max_bitrate = 0;
+
+	can_transceiver_phy = devm_kzalloc(dev, sizeof(struct can_transceiver_phy), GFP_KERNEL);
+
+	match = of_match_node(can_transceiver_phy_ids, pdev->dev.of_node);
+	drvdata = match->data;
+
+	phy = devm_phy_create(dev, dev->of_node,
+			      &can_transceiver_phy_ops);
+	if (IS_ERR(phy)) {
+		dev_err(dev, "failed to create can transceiver phy\n");
+		return PTR_ERR(phy);
+	}
+
+	device_property_read_u32(dev, "max-bitrate", &max_bitrate);
+	phy->attrs.max_link_rate = max_bitrate / 1000000;
+
+	can_transceiver_phy->generic_phy = phy;
+
+	if (drvdata->flags & STB_PRESENT) {
+		standby_gpio = devm_gpiod_get(dev, "standby",   GPIOD_OUT_LOW);
+		if (IS_ERR(standby_gpio))
+			return PTR_ERR(standby_gpio);
+		can_transceiver_phy->standby_gpio = standby_gpio;
+	}
+
+	if (drvdata->flags & EN_PRESENT) {
+		enable_gpio = devm_gpiod_get(dev, "enable",   GPIOD_OUT_LOW);
+		if (IS_ERR(enable_gpio))
+			return PTR_ERR(enable_gpio);
+		can_transceiver_phy->enable_gpio = enable_gpio;
+	}
+
+	phy_set_drvdata(can_transceiver_phy->generic_phy, can_transceiver_phy);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static struct platform_driver can_transceiver_phy_driver = {
+	.probe = can_transceiver_phy_probe,
+	.driver = {
+		.name = "can-transceiver-phy",
+		.of_match_table = can_transceiver_phy_ids,
+	},
+};
+
+module_platform_driver(can_transceiver_phy_driver);
+
+MODULE_AUTHOR("Faiz Abbas <faiz_abbas@ti.com>");
+MODULE_AUTHOR("Aswath Govindraju <a-govindraju@ti.com>");
+MODULE_DESCRIPTION("CAN TRANSCEIVER PHY driver");
+MODULE_LICENSE("GPL v2");