Messages in this thread | | | Date | Fri, 12 Nov 2021 09:40:27 +0100 | From | Marc Kleine-Budde <> | Subject | Re: [PATCH RFC 2/2] phy: phy-can-transceiver: Add support for setting mux |
| |
On 11.11.2021 22:13:12, 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 | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c > index 6f3fe37dee0e..3d8da5226e27 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,22 @@ 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, 1);
Hard coding the "1" looks wrong here. I have seen some boards where you can select between a CAN-2.0 and a single wire CAN transceiver with a mux. So I think we cannot hard code the "1" here.
> + 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 +55,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 +107,15 @@ 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")) {
Is this the proper way of doing this? Looks like we need a devm_mux_control_get_optional(), which doesn't return a -ENODEV if the device doesn't exist.
Cc'ed Peter Rosin.
> + struct mux_control *control; > + > + control = devm_mux_control_get(dev, NULL); > + if (IS_ERR(control)) > + return PTR_ERR(control);
What about making use of dev_err_probe()?
> + 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
-- 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 | [unhandled content-type:application/pgp-signature] | |