Re: [PATCH v53 2/6] can: mcp25xxfd: add regmap infrastructure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/22/20 2:13 PM, Mark Brown wrote:
> On Tue, Sep 22, 2020 at 07:57:49AM +0200, Marc Kleine-Budde wrote:
>> On 9/21/20 9:33 PM, Mark Brown wrote:
> 
>>>> +	if (priv->map_buf_nocrc_rx) {
>>>> +		devm_kfree(&priv->spi->dev, priv->map_buf_nocrc_rx);
>>>> +		priv->map_buf_nocrc_rx = NULL;
>>>> +	}
> 
>>> Why explicitly free managed allocations like this?
> 
>> The driver covers two chips, the mcp2517fd and the mcp2518fd. Due to erratas we
>> need different quirks for these variants. Further you can configure if regmap
>> should use transfers with or without CRC checks (separately for register access,
>> RAM access in RX and TX path).
> 
>> It's possible to distinguish between chip variants by reading/writing a
>> register, so we first init the regmap with CRC mode, autodetect the chip
>> variant, and then re-init the regmap with the quirks of that detected variant.
> 
>> The mcp25xxfd regmap init is written so that resources that are allocated first,
>> but not needed due to different quirks are then freed (both regmap and the
>> buffers for rx/tx).
> 
> This feels like a non-idiomatic way of doing this - usually you'd
> enumerate then allocate the extra maps (using regmap_reinit_cache() to
> replace the regmap used to do the enumeration).

I have implemented two regmap clients for this driver. One does transfers with
CRC, the other one not. Both are REGCACHE_NONE.

For autodetection the driver initializes the regmap which does transfers with
CRC. Then it detects the chip variant and maybe that variant works properly
without CRC. The CRC regmap is not needed anymore, so the buffer used to
linearize the data, is freed.

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 |

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux