On 11.07.2023 16:03:55, Michal Simek wrote:
> From: Srinivas Neeli <srinivas.neeli@xxxxxxx>
>
> Add support for an optional reset for the CAN controller using the reset
> driver. If the CAN node contains the "resets" property, then this logic
> will perform CAN controller reset.
>
> Signed-off-by: Srinivas Neeli <srinivas.neeli@xxxxxxx>
> Signed-off-by: Michal Simek <michal.simek@xxxxxxx>
> ---
>
> drivers/net/can/xilinx_can.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
> index 4d3283db3a13..929e00061898 100644
> --- a/drivers/net/can/xilinx_can.c
> +++ b/drivers/net/can/xilinx_can.c
> @@ -30,6 +30,7 @@
> #include <linux/can/error.h>
> #include <linux/phy/phy.h>
> #include <linux/pm_runtime.h>
> +#include <linux/reset.h>
>
> #define DRIVER_NAME "xilinx_can"
>
> @@ -200,6 +201,7 @@ struct xcan_devtype_data {
> * @can_clk: Pointer to struct clk
> * @devtype: Device type specific constants
> * @transceiver: Optional pointer to associated CAN transceiver
> + * @rstc: Pointer to reset control
> */
> struct xcan_priv {
> struct can_priv can;
> @@ -218,6 +220,7 @@ struct xcan_priv {
> struct clk *can_clk;
> struct xcan_devtype_data devtype;
> struct phy *transceiver;
> + struct reset_control *rstc;
> };
>
> /* CAN Bittiming constants as per Xilinx CAN specs */
> @@ -1799,6 +1802,16 @@ static int xcan_probe(struct platform_device *pdev)
> priv->can.do_get_berr_counter = xcan_get_berr_counter;
> priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> CAN_CTRLMODE_BERR_REPORTING;
> + priv->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
> + if (IS_ERR(priv->rstc)) {
> + dev_err(&pdev->dev, "Cannot get CAN reset.\n");
> + ret = PTR_ERR(priv->rstc);
> + goto err_free;
> + }
> +
> + ret = reset_control_reset(priv->rstc);
> + if (ret)
> + goto err_free;
>
> if (devtype->cantype == XAXI_CANFD) {
> priv->can.data_bittiming_const =
> @@ -1827,7 +1840,7 @@ static int xcan_probe(struct platform_device *pdev)
> /* Get IRQ for the device */
> ret = platform_get_irq(pdev, 0);
> if (ret < 0)
> - goto err_free;
> + goto err_reset;
>
> ndev->irq = ret;
>
> @@ -1843,21 +1856,21 @@ static int xcan_probe(struct platform_device *pdev)
> if (IS_ERR(priv->can_clk)) {
> ret = dev_err_probe(&pdev->dev, PTR_ERR(priv->can_clk),
> "device clock not found\n");
> - goto err_free;
> + goto err_reset;
> }
>
> priv->bus_clk = devm_clk_get(&pdev->dev, devtype->bus_clk_name);
> if (IS_ERR(priv->bus_clk)) {
> ret = dev_err_probe(&pdev->dev, PTR_ERR(priv->bus_clk),
> "bus clock not found\n");
> - goto err_free;
> + goto err_reset;
> }
>
> 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;
> + goto err_reset;
> }
> priv->transceiver = transceiver;
>
> @@ -1904,6 +1917,8 @@ static int xcan_probe(struct platform_device *pdev)
> err_disableclks:
> pm_runtime_put(priv->dev);
> pm_runtime_disable(&pdev->dev);
> +err_reset:
> + reset_control_assert(priv->rstc);
> err_free:
> free_candev(ndev);
> err:
> @@ -1920,10 +1935,12 @@ static int xcan_probe(struct platform_device *pdev)
> static void xcan_remove(struct platform_device *pdev)
> {
> struct net_device *ndev = platform_get_drvdata(pdev);
> + struct xcan_priv *priv = netdev_priv(ndev);
>
> unregister_candev(ndev);
> pm_runtime_disable(&pdev->dev);
> free_candev(ndev);
> + reset_control_assert(priv->rstc);
Nitpick: Can you make this symmetric with respect to the error handling
in xcan_probe()?
Oh - that's not a cosmetic issue, it's a use-after-free. free_candev()
frees your priv.
> }
>
> static struct platform_driver xcan_driver = {
> --
> 2.36.1
>
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Attachment:
signature.asc
Description: PGP signature
