linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Mark Brown <broonie@kernel.org>
Cc: linux-can@vger.kernel.org, manivannan.sadhasivam@linaro.org,
	dev.kurt@vandijck-laurijssen.be
Subject: Re: [PATCH v53 2/6] can: mcp25xxfd: add regmap infrastructure
Date: Tue, 22 Sep 2020 15:56:38 +0200	[thread overview]
Message-ID: <e483ba1f-5958-4ba6-e82e-be611247ccd1@pengutronix.de> (raw)
In-Reply-To: <20200922121305.GT4792@sirena.org.uk>


[-- Attachment #1.1: Type: text/plain, Size: 1960 bytes --]

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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2020-09-22 13:56 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-18 17:25 [PATCH v53 1/6] mcp25xxfd: initial driver support Marc Kleine-Budde
2020-09-18 17:25 ` [PATCH v53 1/6] dt-binding: can: mcp25xxfd: document device tree bindings Marc Kleine-Budde
2020-09-18 17:25 ` [PATCH v53 2/6] can: mcp25xxfd: add regmap infrastructure Marc Kleine-Budde
2020-09-21 19:33   ` Mark Brown
2020-09-22  5:57     ` Marc Kleine-Budde
2020-09-22 12:13       ` Mark Brown
2020-09-22 13:56         ` Marc Kleine-Budde [this message]
2020-09-22 14:23           ` Mark Brown
2020-09-18 17:25 ` [PATCH v53 3/6] can: mcp25xxfd: add driver for Microchip MCP25xxFD SPI CAN Marc Kleine-Budde
2020-09-18 17:25 ` [PATCH v53 4/6] can: mcp25xxfd: add listen-only mode Marc Kleine-Budde
2020-09-18 17:25 ` [PATCH v53 5/6] MAINTAINERS: Add entry for Microchip MCP25XXFD SPI-CAN network driver Marc Kleine-Budde
2020-09-18 17:25 ` [PATCH v53 6/6] MAINTAINERS: Add reviewer entry for microchip mcp25xxfd " Marc Kleine-Budde

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e483ba1f-5958-4ba6-e82e-be611247ccd1@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=broonie@kernel.org \
    --cc=dev.kurt@vandijck-laurijssen.be \
    --cc=linux-can@vger.kernel.org \
    --cc=manivannan.sadhasivam@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).