Hi Marcel, Style check only (I am not yet familiar enough with the devicetree). On Mon. 17 Apr. 2023 at 18:01, Marcel Hellwig <git@xxxxxxxxxxxxx> wrote: > Currently the xilinx_can driver does not support adding a phy like the > "ti,tcan1043" to its devicetree. > > This code makes it possible to add such phy, so that the kernel makes > sure that the PHY is in operational state, when the link is set to an > "up" state. > > Signed-off-by: Marcel Hellwig <git@xxxxxxxxxxxxx> > --- > drivers/net/can/xilinx_can.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c > index 43c812ea1de0..6a5b805d579a 100644 > --- a/drivers/net/can/xilinx_can.c > +++ b/drivers/net/can/xilinx_can.c > @@ -28,6 +28,7 @@ > #include <linux/types.h> > #include <linux/can/dev.h> > #include <linux/can/error.h> > +#include <linux/phy/phy.h> > #include <linux/pm_runtime.h> > > #define DRIVER_NAME "xilinx_can" > @@ -215,6 +216,7 @@ struct xcan_priv { > struct clk *bus_clk; > struct clk *can_clk; > struct xcan_devtype_data devtype; > + struct phy *transceiver; > }; > > /* CAN Bittiming constants as per Xilinx CAN specs */ > @@ -1419,6 +1421,12 @@ static int xcan_open(struct net_device *ndev) > struct xcan_priv *priv = netdev_priv(ndev); > int ret; > > + ret = phy_power_on(priv->transceiver); > + if (ret) { > + netdev_err(ndev, "%s: phy_power_on failed(%d)\n", __func__, ret); >From the Linux kernel coding style: Printing numbers in parentheses (%d) adds no value and should be avoided. Link: https://www.kernel.org/doc/html/latest/process/coding-style.html#printing-kernel-messages Also consider %pe to print the mnemotechnic instead of the value: netdev_err(ndev, "%s: phy_power_on failed: %pe\n", __func__, ERR_PTR(ret)); > + return ret; > + } > + > ret = pm_runtime_get_sync(priv->dev); > if (ret < 0) { > netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n", It is up to you, but bonus points if you send a clean-up patch to fix the existing log messages. > @@ -1461,6 +1469,7 @@ static int xcan_open(struct net_device *ndev) > err_irq: > free_irq(ndev->irq, ndev); > err: > + phy_power_off(priv->transceiver); > pm_runtime_put(priv->dev); > > return ret; > @@ -1482,6 +1491,7 @@ static int xcan_close(struct net_device *ndev) > free_irq(ndev->irq, ndev); > close_candev(ndev); > > + phy_power_off(priv->transceiver); > pm_runtime_put(priv->dev); > > return 0; > @@ -1713,6 +1723,7 @@ static int xcan_probe(struct platform_device *pdev) > { > struct net_device *ndev; > struct xcan_priv *priv; > + struct phy *transceiver; > const struct of_device_id *of_id; > const struct xcan_devtype_data *devtype = &xcan_axi_data; > void __iomem *addr; > @@ -1843,6 +1854,14 @@ static int xcan_probe(struct platform_device *pdev) > goto err_free; > } > > + transceiver = devm_phy_optional_get(&pdev->dev, NULL); > + if (IS_ERR(transceiver)) { > + ret = PTR_ERR(transceiver); > + dev_err_probe(&pdev->dev, ret, "failed to get phy\n"); > + goto err_free; > + } > + priv->transceiver = transceiver; > + > priv->write_reg = xcan_write_reg_le; > priv->read_reg = xcan_read_reg_le; > > @@ -1869,6 +1888,7 @@ static int xcan_probe(struct platform_device *pdev) > goto err_disableclks; > } > > + of_can_transceiver(ndev); > pm_runtime_put(&pdev->dev); > > if (priv->devtype.flags & XCAN_FLAG_CANFD_2) { > -- > 2.34.1 >