Re: [PATCH 2/2] can: sja1000: of: add compatibility with Technologic Systems version

[Marc Kleine-Budde]

On 12/18/2015 09:17 PM, Damien Riegel wrote:
> Technologic Systems provides an IP compatible with the SJA1000,
> instantiated in an FPGA. Because of some bus widths issue, access to
> registers is made through a "window" that works like this:
> 
>     base + 0x0: address to read/write
>     base + 0x2: 8-bit register value
> 
> This commit adds a new compatible device, "technologic,sja1000", with
> read and write functions using the window mechanism.
> 
> Signed-off-by: Damien Riegel <damien.riegel@xxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/net/can/sja1000/sja1000_platform.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
> index 0552ed4..6cbf251 100644
> --- a/drivers/net/can/sja1000/sja1000_platform.c
> +++ b/drivers/net/can/sja1000/sja1000_platform.c
> @@ -70,6 +70,18 @@ static void sp_write_reg32(const struct sja1000_priv *priv, int reg, u8 val)
>  	iowrite8(val, priv->reg_base + reg * 4);
>  }
>  
> +static u8 ts4800_read_reg16(const struct sja1000_priv *priv, int reg)
> +{
> +	sp_write_reg16(priv, 0,  reg);
> +	return sp_read_reg16(priv, 2);

This is racy, please add a spinlock.

> +}
> +
> +static void ts4800_write_reg16(const struct sja1000_priv *priv, int reg, u8 val)
> +{
> +	sp_write_reg16(priv, 0, reg);
> +	sp_write_reg16(priv, 2, val);

This is racy, too.

Have a look at https://marc.info/?l=linux-can&m=137149497403825&w=2

Marc

> +}
> +
>  static void sp_populate(struct sja1000_priv *priv,
>  			struct sja1000_platform_data *pdata,
>  			unsigned long resource_mem_flags)
> @@ -98,21 +110,34 @@ static void sp_populate(struct sja1000_priv *priv,
>  
>  static void sp_populate_of(struct sja1000_priv *priv, struct device_node *of)
>  {
> +	int is_technologic;
>  	int err;
>  	u32 prop;
>  
> +	is_technologic = of_device_is_compatible(of, "technologic,sja1000");
> +
>  	err = of_property_read_u32(of, "reg-io-width", &prop);
>  	if (err)
>  		prop = 1; /* 8 bit is default */
>  
> +	if (is_technologic && prop != 2) {
> +		netdev_warn(priv->dev, "forcing reg-io-width to 2\n");
> +		prop = 2;
> +	}
> +
>  	switch (prop) {
>  	case 4:
>  		priv->read_reg = sp_read_reg32;
>  		priv->write_reg = sp_write_reg32;
>  		break;
>  	case 2:
> -		priv->read_reg = sp_read_reg16;
> -		priv->write_reg = sp_write_reg16;
> +		if (is_technologic) {
> +			priv->read_reg = ts4800_read_reg16;
> +			priv->write_reg = ts4800_write_reg16;
> +		} else {
> +			priv->read_reg = sp_read_reg16;
> +			priv->write_reg = sp_write_reg16;
> +		}
>  		break;
>  	case 1:	/* fallthrough */
>  	default:
> @@ -244,6 +269,7 @@ static int sp_remove(struct platform_device *pdev)
>  
>  static const struct of_device_id sp_of_table[] = {
>  	{.compatible = "nxp,sja1000"},
> +	{.compatible = "technologic,sja1000"},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, sp_of_table);
> 


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

Attachment: signature.asc
Description: OpenPGP digital signature