| Message ID | 20211122125624.6431-5-a-govindraju@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MUX: Add support for reading enable state from DT | expand |
On 22.11.2021 18:26:24, Aswath Govindraju wrote: > On some boards, for routing CAN signals from controller to transceiver, > muxes might need to be set. Therefore, add support for setting the mux by > reading the mux-controls property from the device tree node. > > Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> > --- > drivers/phy/phy-can-transceiver.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c > index 6f3fe37dee0e..15056b9d68ba 100644 > --- a/drivers/phy/phy-can-transceiver.c > +++ b/drivers/phy/phy-can-transceiver.c > @@ -10,6 +10,7 @@ > #include<linux/module.h> > #include<linux/gpio.h> > #include<linux/gpio/consumer.h> > +#include <linux/mux/consumer.h> > > struct can_transceiver_data { > u32 flags; > @@ -21,13 +22,23 @@ struct can_transceiver_phy { > struct phy *generic_phy; > struct gpio_desc *standby_gpio; > struct gpio_desc *enable_gpio; > + struct mux_control *mux_ctrl; > }; > > /* Power on function */ > static int can_transceiver_phy_power_on(struct phy *phy) > { > + int ret; > struct can_transceiver_phy *can_transceiver_phy = phy_get_drvdata(phy); > > + if (can_transceiver_phy->mux_ctrl) { > + ret = mux_control_select(can_transceiver_phy->mux_ctrl, > + mux_control_enable_state(can_transceiver_phy->mux_ctrl)); > + if (ret) { > + dev_err(&phy->dev, "Failed to select CAN mux: %d\n", ret); > + return ret; > + } > + } > if (can_transceiver_phy->standby_gpio) > gpiod_set_value_cansleep(can_transceiver_phy->standby_gpio, 0); > if (can_transceiver_phy->enable_gpio) > @@ -45,6 +56,8 @@ static int can_transceiver_phy_power_off(struct phy *phy) > 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); > + if (can_transceiver_phy->mux_ctrl) > + mux_control_deselect(can_transceiver_phy->mux_ctrl); > > return 0; > } > @@ -95,6 +108,19 @@ static int can_transceiver_phy_probe(struct platform_device *pdev) > match = of_match_node(can_transceiver_phy_ids, pdev->dev.of_node); > drvdata = match->data; > > + if (of_property_read_bool(dev->of_node, "mux-controls")) { > + struct mux_control *control; > + int ret; > + > + control = devm_mux_control_get(dev, NULL); > + if (IS_ERR(control)) { > + ret = PTR_ERR(control); > + dev_err_probe(&pdev->dev, ret, "failed to get mux\n"); > + return PTR_ERR(control); > + } if (IS_ERR(control)) return dev_err_probe(&pdev, PTR_ERR(control), "failed to get mux\n"); > + can_transceiver_phy->mux_ctrl = control; > + } > + > phy = devm_phy_create(dev, dev->of_node, > &can_transceiver_phy_ops); > if (IS_ERR(phy)) { > -- > 2.17.1 > > regards, Marc
On 22.11.2021 18:26:24, Aswath Govindraju wrote: > On some boards, for routing CAN signals from controller to transceiver, > muxes might need to be set. Therefore, add support for setting the mux by > reading the mux-controls property from the device tree node. > > Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> > --- > drivers/phy/phy-can-transceiver.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c > index 6f3fe37dee0e..15056b9d68ba 100644 > --- a/drivers/phy/phy-can-transceiver.c > +++ b/drivers/phy/phy-can-transceiver.c > @@ -10,6 +10,7 @@ > #include<linux/module.h> > #include<linux/gpio.h> > #include<linux/gpio/consumer.h> > +#include <linux/mux/consumer.h> > > struct can_transceiver_data { > u32 flags; > @@ -21,13 +22,23 @@ struct can_transceiver_phy { > struct phy *generic_phy; > struct gpio_desc *standby_gpio; > struct gpio_desc *enable_gpio; > + struct mux_control *mux_ctrl; > }; > > /* Power on function */ > static int can_transceiver_phy_power_on(struct phy *phy) > { > + int ret; > struct can_transceiver_phy *can_transceiver_phy = phy_get_drvdata(phy); > > + if (can_transceiver_phy->mux_ctrl) { > + ret = mux_control_select(can_transceiver_phy->mux_ctrl, > + mux_control_enable_state(can_transceiver_phy->mux_ctrl)); > + if (ret) { > + dev_err(&phy->dev, "Failed to select CAN mux: %d\n", ret); > + return ret; > + } > + } > if (can_transceiver_phy->standby_gpio) > gpiod_set_value_cansleep(can_transceiver_phy->standby_gpio, 0); > if (can_transceiver_phy->enable_gpio) > @@ -45,6 +56,8 @@ static int can_transceiver_phy_power_off(struct phy *phy) > 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); > + if (can_transceiver_phy->mux_ctrl) > + mux_control_deselect(can_transceiver_phy->mux_ctrl); > > return 0; > } > @@ -95,6 +108,19 @@ static int can_transceiver_phy_probe(struct platform_device *pdev) > match = of_match_node(can_transceiver_phy_ids, pdev->dev.of_node); > drvdata = match->data; > > + if (of_property_read_bool(dev->of_node, "mux-controls")) { > + struct mux_control *control; > + int ret; > + > + control = devm_mux_control_get(dev, NULL); > + if (IS_ERR(control)) { > + ret = PTR_ERR(control); > + dev_err_probe(&pdev->dev, ret, "failed to get mux\n"); > + return PTR_ERR(control); > + } > + can_transceiver_phy->mux_ctrl = control; > + } What about adding a devm_mux_control_get_optional(), which doesn't return a -ENODEV but a NULL pointer if the device doesn't exist? Marc
Hi Marc, On 22/11/21 6:42 pm, Marc Kleine-Budde wrote: > On 22.11.2021 18:26:24, Aswath Govindraju wrote: >> On some boards, for routing CAN signals from controller to transceiver, >> muxes might need to be set. Therefore, add support for setting the mux by >> reading the mux-controls property from the device tree node. >> >> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> >> --- >> drivers/phy/phy-can-transceiver.c | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c >> index 6f3fe37dee0e..15056b9d68ba 100644 >> --- a/drivers/phy/phy-can-transceiver.c >> +++ b/drivers/phy/phy-can-transceiver.c >> @@ -10,6 +10,7 @@ >> #include<linux/module.h> >> #include<linux/gpio.h> >> #include<linux/gpio/consumer.h> >> +#include <linux/mux/consumer.h> >> >> struct can_transceiver_data { >> u32 flags; >> @@ -21,13 +22,23 @@ struct can_transceiver_phy { >> struct phy *generic_phy; >> struct gpio_desc *standby_gpio; >> struct gpio_desc *enable_gpio; >> + struct mux_control *mux_ctrl; >> }; >> >> /* Power on function */ >> static int can_transceiver_phy_power_on(struct phy *phy) >> { >> + int ret; >> struct can_transceiver_phy *can_transceiver_phy = phy_get_drvdata(phy); >> >> + if (can_transceiver_phy->mux_ctrl) { >> + ret = mux_control_select(can_transceiver_phy->mux_ctrl, >> + mux_control_enable_state(can_transceiver_phy->mux_ctrl)); >> + if (ret) { >> + dev_err(&phy->dev, "Failed to select CAN mux: %d\n", ret); >> + return ret; >> + } >> + } >> if (can_transceiver_phy->standby_gpio) >> gpiod_set_value_cansleep(can_transceiver_phy->standby_gpio, 0); >> if (can_transceiver_phy->enable_gpio) >> @@ -45,6 +56,8 @@ static int can_transceiver_phy_power_off(struct phy *phy) >> 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); >> + if (can_transceiver_phy->mux_ctrl) >> + mux_control_deselect(can_transceiver_phy->mux_ctrl); >> >> return 0; >> } >> @@ -95,6 +108,19 @@ static int can_transceiver_phy_probe(struct platform_device *pdev) >> match = of_match_node(can_transceiver_phy_ids, pdev->dev.of_node); >> drvdata = match->data; >> >> + if (of_property_read_bool(dev->of_node, "mux-controls")) { >> + struct mux_control *control; >> + int ret; >> + >> + control = devm_mux_control_get(dev, NULL); >> + if (IS_ERR(control)) { >> + ret = PTR_ERR(control); >> + dev_err_probe(&pdev->dev, ret, "failed to get mux\n"); >> + return PTR_ERR(control); >> + } >> + can_transceiver_phy->mux_ctrl = control; >> + } > > What about adding a devm_mux_control_get_optional(), which doesn't > return a -ENODEV but a NULL pointer if the device doesn't exist? > I tried adding it in the following manner, +/** + * devm_mux_control_optional_get() - Optionally get the mux-control for a + * device, with resource management. + * @dev: The device that needs a mux-control. + * @mux_name: The name identifying the mux-control. + * + * This differs from devm_mux_control_get in that if the mux does not + * exist, it is not considered an error and -ENODEV will not be + * returned. Instead the NULL is returned. + * + * Return: Pointer to the mux-control, or an ERR_PTR with a negative errno. + */ +struct mux_control *devm_mux_control_optional_get(struct device *dev, + const char *mux_name) +{ + struct mux_control *mux_ctrl; + + mux_ctrl = devm_mux_control_get(dev, mux_name); + if (PTR_ERR(mux_ctrl) == -ENOENT) + mux_ctrl = NULL; + + return mux_ctrl; +} +EXPORT_SYMBOL_GPL(devm_mux_control_optional_get); + However the issue is that there is a print in mux_control_get() dev_err(dev, "%pOF: failed to get mux-control %s(%i)\n", which is getting printed, whenever mux-controls property is not found. Therefore, I was not sure about how to go about this issue and did not implement it. Thanks, Aswath > Marc >
On 22.11.2021 18:50:00, Aswath Govindraju wrote: > > What about adding a devm_mux_control_get_optional(), which doesn't > > return a -ENODEV but a NULL pointer if the device doesn't exist? > > > > I tried adding it in the following manner, > > +/** > + * devm_mux_control_optional_get() - Optionally get the mux-control for a > + * device, with resource management. > + * @dev: The device that needs a mux-control. > + * @mux_name: The name identifying the mux-control. > + * > + * This differs from devm_mux_control_get in that if the mux does not > + * exist, it is not considered an error and -ENODEV will not be > + * returned. Instead the NULL is returned. > + * > + * Return: Pointer to the mux-control, or an ERR_PTR with a negative errno. > + */ > +struct mux_control *devm_mux_control_optional_get(struct device *dev, > + const char *mux_name) > +{ > + struct mux_control *mux_ctrl; > + > + mux_ctrl = devm_mux_control_get(dev, mux_name); > + if (PTR_ERR(mux_ctrl) == -ENOENT) > + mux_ctrl = NULL; > + > + return mux_ctrl; > +} > +EXPORT_SYMBOL_GPL(devm_mux_control_optional_get); > + > > However the issue is that there is a print in mux_control_get() > dev_err(dev, "%pOF: failed to get mux-control %s(%i)\n", > > which is getting printed, whenever mux-controls property is not found. > Therefore, I was not sure about how to go about this issue and did not > implement it. Ok, this would require more tweaking in the mux layer. Then leave it as is. regards, Marc
Hi Aswath, On 22/11/21 6:26 pm, Aswath Govindraju wrote: > On some boards, for routing CAN signals from controller to transceiver, > muxes might need to be set. Therefore, add support for setting the mux by > reading the mux-controls property from the device tree node. > > Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> > --- > drivers/phy/phy-can-transceiver.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c > index 6f3fe37dee0e..15056b9d68ba 100644 > --- a/drivers/phy/phy-can-transceiver.c > +++ b/drivers/phy/phy-can-transceiver.c > @@ -10,6 +10,7 @@ > #include<linux/module.h> > #include<linux/gpio.h> > #include<linux/gpio/consumer.h> > +#include <linux/mux/consumer.h> > > struct can_transceiver_data { > u32 flags; > @@ -21,13 +22,23 @@ struct can_transceiver_phy { > struct phy *generic_phy; > struct gpio_desc *standby_gpio; > struct gpio_desc *enable_gpio; > + struct mux_control *mux_ctrl; > }; > > /* Power on function */ > static int can_transceiver_phy_power_on(struct phy *phy) > { > + int ret; > struct can_transceiver_phy *can_transceiver_phy = phy_get_drvdata(phy); > > + if (can_transceiver_phy->mux_ctrl) { > + ret = mux_control_select(can_transceiver_phy->mux_ctrl, > + mux_control_enable_state(can_transceiver_phy->mux_ctrl)); Would need 'select MULTIPLEXER' in Kconfig. Thanks, Kishon > + if (ret) { > + dev_err(&phy->dev, "Failed to select CAN mux: %d\n", ret); > + return ret; > + } > + } > if (can_transceiver_phy->standby_gpio) > gpiod_set_value_cansleep(can_transceiver_phy->standby_gpio, 0); > if (can_transceiver_phy->enable_gpio) > @@ -45,6 +56,8 @@ static int can_transceiver_phy_power_off(struct phy *phy) > 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); > + if (can_transceiver_phy->mux_ctrl) > + mux_control_deselect(can_transceiver_phy->mux_ctrl); > > return 0; > } > @@ -95,6 +108,19 @@ static int can_transceiver_phy_probe(struct platform_device *pdev) > match = of_match_node(can_transceiver_phy_ids, pdev->dev.of_node); > drvdata = match->data; > > + if (of_property_read_bool(dev->of_node, "mux-controls")) { > + struct mux_control *control; > + int ret; > + > + control = devm_mux_control_get(dev, NULL); > + if (IS_ERR(control)) { > + ret = PTR_ERR(control); > + dev_err_probe(&pdev->dev, ret, "failed to get mux\n"); > + return PTR_ERR(control); > + } > + can_transceiver_phy->mux_ctrl = control; > + } > + > phy = devm_phy_create(dev, dev->of_node, > &can_transceiver_phy_ops); > if (IS_ERR(phy)) { >
diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c index 6f3fe37dee0e..15056b9d68ba 100644 --- a/drivers/phy/phy-can-transceiver.c +++ b/drivers/phy/phy-can-transceiver.c @@ -10,6 +10,7 @@ #include<linux/module.h> #include<linux/gpio.h> #include<linux/gpio/consumer.h> +#include <linux/mux/consumer.h> struct can_transceiver_data { u32 flags; @@ -21,13 +22,23 @@ struct can_transceiver_phy { struct phy *generic_phy; struct gpio_desc *standby_gpio; struct gpio_desc *enable_gpio; + struct mux_control *mux_ctrl; }; /* Power on function */ static int can_transceiver_phy_power_on(struct phy *phy) { + int ret; struct can_transceiver_phy *can_transceiver_phy = phy_get_drvdata(phy); + if (can_transceiver_phy->mux_ctrl) { + ret = mux_control_select(can_transceiver_phy->mux_ctrl, + mux_control_enable_state(can_transceiver_phy->mux_ctrl)); + if (ret) { + dev_err(&phy->dev, "Failed to select CAN mux: %d\n", ret); + return ret; + } + } if (can_transceiver_phy->standby_gpio) gpiod_set_value_cansleep(can_transceiver_phy->standby_gpio, 0); if (can_transceiver_phy->enable_gpio) @@ -45,6 +56,8 @@ static int can_transceiver_phy_power_off(struct phy *phy) 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); + if (can_transceiver_phy->mux_ctrl) + mux_control_deselect(can_transceiver_phy->mux_ctrl); return 0; } @@ -95,6 +108,19 @@ static int can_transceiver_phy_probe(struct platform_device *pdev) match = of_match_node(can_transceiver_phy_ids, pdev->dev.of_node); drvdata = match->data; + if (of_property_read_bool(dev->of_node, "mux-controls")) { + struct mux_control *control; + int ret; + + control = devm_mux_control_get(dev, NULL); + if (IS_ERR(control)) { + ret = PTR_ERR(control); + dev_err_probe(&pdev->dev, ret, "failed to get mux\n"); + return PTR_ERR(control); + } + can_transceiver_phy->mux_ctrl = control; + } + phy = devm_phy_create(dev, dev->of_node, &can_transceiver_phy_ops); if (IS_ERR(phy)) {
On some boards, for routing CAN signals from controller to transceiver, muxes might need to be set. Therefore, add support for setting the mux by reading the mux-controls property from the device tree node. Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> --- drivers/phy/phy-can-transceiver.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)