[PATCH v2 2/3] can: sun4i_can: add support for R40 CAN controller

[Andre Przywara]

On Tue, 11 Jan 2022 18:57:08 +0300
Evgeny Boger <boger at wirenboard.com> wrote:

Hi,

> Allwinner R40 (also known as A40i, T3, V40) has a CAN controller. The
> controller is the same as in earlier A10 and A20 SoCs, but needs reset
> line to be deasserted before use.
> 
> This patch adds a new compatible for R40 CAN controller. Depending
> on the compatible, reset line can be requested from DT.
> 
> Signed-off-by: Evgeny Boger <boger at wirenboard.com>
> ---
>  drivers/net/can/sun4i_can.c | 61 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/sun4i_can.c b/drivers/net/can/sun4i_can.c
> index 54aa7c25c4de..24a61326915e 100644
> --- a/drivers/net/can/sun4i_can.c
> +++ b/drivers/net/can/sun4i_can.c
> @@ -61,6 +61,7 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
> +#include <linux/reset.h>
>  
>  #define DRV_NAME "sun4i_can"
>  
> @@ -200,10 +201,20 @@
>  #define SUN4I_CAN_MAX_IRQ	20
>  #define SUN4I_MODE_MAX_RETRIES	100
>  
> +/**
> + * struct sun4ican_quirks - Differences between SoC variants.
> + *
> + * @has_reset: SoC needs reset deasserted.
> + */
> +struct sun4ican_quirks {
> +	bool		has_reset;
> +};
> +
>  struct sun4ican_priv {
>  	struct can_priv can;
>  	void __iomem *base;
>  	struct clk *clk;
> +	struct reset_control *reset;
>  	spinlock_t cmdreg_lock;	/* lock for concurrent cmd register writes */
>  };
>  
> @@ -702,6 +713,13 @@ static int sun4ican_open(struct net_device *dev)
>  		goto exit_irq;
>  	}
>  
> +	/* software reset deassert */
> +	err = reset_control_deassert(priv->reset);
> +	if (err) {
> +		netdev_err(dev, "could not deassert CAN reset\n");
> +		goto exit_soft_reset;
> +	}
> +
>  	/* turn on clocking for CAN peripheral block */
>  	err = clk_prepare_enable(priv->clk);
>  	if (err) {
> @@ -723,6 +741,8 @@ static int sun4ican_open(struct net_device *dev)
>  exit_can_start:
>  	clk_disable_unprepare(priv->clk);
>  exit_clock:
> +	reset_control_assert(priv->reset);
> +exit_soft_reset:
>  	free_irq(dev->irq, dev);
>  exit_irq:
>  	close_candev(dev);
> @@ -736,6 +756,7 @@ static int sun4ican_close(struct net_device *dev)
>  	netif_stop_queue(dev);
>  	sun4i_can_stop(dev);
>  	clk_disable_unprepare(priv->clk);
> +	reset_control_assert(priv->reset);
>  
>  	free_irq(dev->irq, dev);
>  	close_candev(dev);
> @@ -750,8 +771,27 @@ static const struct net_device_ops sun4ican_netdev_ops = {
>  	.ndo_start_xmit = sun4ican_start_xmit,
>  };
>  
> +static const struct sun4ican_quirks sun4ican_quirks_a10 = {
> +	.has_reset = false,
> +};
> +
> +static const struct sun4ican_quirks sun4ican_quirks_r40 = {
> +	.has_reset = true,
> +};
> +
>  static const struct of_device_id sun4ican_of_match[] = {
> -	{.compatible = "allwinner,sun4i-a10-can"},
> +	{
> +		.compatible = "allwinner,sun4i-a10-can",
> +		.data = &sun4ican_quirks_a10
> +	},
> +	{
> +		.compatible = "allwinner,sun7i-a20-can",
> +		.data = &sun4ican_quirks_a10
> +	},

This one looks unnecessary, the binding says that it must be paired with
the A10 fallback.
We should only add an A20 compatible match if there is really a reason,
otherwise it encourages people to produce DTs with *only* the A20
compatible (violating the binding).

Cheers,
Andre

> +	{
> +		.compatible = "allwinner,sun8i-r40-can",
> +		.data = &sun4ican_quirks_r40
> +	},
>  	{},
>  };
>  
> @@ -771,10 +811,28 @@ static int sun4ican_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
>  	struct clk *clk;
> +	struct reset_control *reset = NULL;
>  	void __iomem *addr;
>  	int err, irq;
>  	struct net_device *dev;
>  	struct sun4ican_priv *priv;
> +	const struct sun4ican_quirks *quirks;
> +
> +	quirks = of_device_get_match_data(&pdev->dev);
> +	if (!quirks) {
> +		dev_err(&pdev->dev, "failed to determine the quirks to use\n");
> +		err = -ENODEV;
> +		goto exit;
> +	}
> +
> +	if (quirks->has_reset) {
> +		reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> +		if (IS_ERR(reset)) {
> +			dev_err(&pdev->dev, "unable to request reset\n");
> +			err = PTR_ERR(reset);
> +			goto exit;
> +		}
> +	}
>  
>  	clk = of_clk_get(np, 0);
>  	if (IS_ERR(clk)) {
> @@ -818,6 +876,7 @@ static int sun4ican_probe(struct platform_device *pdev)
>  				       CAN_CTRLMODE_3_SAMPLES;
>  	priv->base = addr;
>  	priv->clk = clk;
> +	priv->reset = reset;
>  	spin_lock_init(&priv->cmdreg_lock);
>  
>  	platform_set_drvdata(pdev, dev);