LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
To: Joakim Zhang <qiangqing.zhang@nxp.com>, Rob Herring <robh@kernel.org>
Cc: "shawnguo@kernel.org" <shawnguo@kernel.org>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	dl-linux-imx <linux-imx@nxp.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V1 1/4] bindings: nvmem: introduce "reverse-data" property
Date: Wed, 8 Sep 2021 09:48:53 +0100	[thread overview]
Message-ID: <a25379a9-621c-3fda-d933-9cc2d2fc706a@linaro.org> (raw)
In-Reply-To: <DB8PR04MB6795046DC70B8238963C343AE6D49@DB8PR04MB6795.eurprd04.prod.outlook.com>



On 08/09/2021 08:14, Joakim Zhang wrote:
> 
> Hi Srinivas,
> 
> [...]
>> I have pushed some nvmem core patches which are just compile tested to
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern
>> el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fsrini%2Fnvmem.git%2Flog%
>> 2F%3Fh%3Dtopic%2Fpost-processing&amp;data=04%7C01%7Cqiangqing.zhan
>> g%40nxp.com%7Cadfa3ba63c634937876308d971e7e71f%7C686ea1d3bc2b4c6
>> fa92cd99c5c301635%7C0%7C0%7C637666063097239185%7CUnknown%7CT
>> WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJ
>> XVCI6Mn0%3D%7C1000&amp;sdata=W9yAnGm9rYzlSZuAAGiN4VHUtKYUTt9S
>> oyGQ9QsY7fI%3D&amp;reserved=0
>>
>> This should provide the callback hook I was talking about.
> 
> Thanks a lot! Yes, this could be more common, vendors can parse their mac
> address for different encoding style, also can extend for other cases.

Yes, that is the idea,
> 
>> Can you take a look at them and let me know if it works for you.
> 
> There are some small issues need to be update:
> 1) https://git.kernel.org/pub/scm/linux/kernel/git/srini/nvmem.git/commit/?h=topic/post-processing&id=624f2cc99b48bbfe05c11e58fb73f84abb1a646e
> 	of_get_property() can't get the cell value, so I change to of_property_read_s32()
> 2) https://git.kernel.org/pub/scm/linux/kernel/git/srini/nvmem.git/commit/?h=topic/post-processing&id=a424302c7b15da41e1e8de56b0c78021b9a96c1e
> 	if (!nvmem->cell_post_process) {} should be if (nvmem->cell_post_process) {}, if we have this callback, we need do the post-processing.
> 
I have pushed these changes now to the branch.

>> I have also added some test changes to imx provider driver as well, which you
>> might have to take a closer look to get it working.
>>
>> You need to look at adding/changing two things:
>>
>> 1. setting reverse_mac_address flag in imx driver.
>> 	Does IMX always has mac-address reversed? if yes then we do not need
>> any new bindings for imx nvmem provider, if no we might need to add some
>> kind of flag to indicate this.
> 
> No, it's depend on how to program the effuse.
> To avoid introducing consumer property in devicetree, I prefer to move reverse_mac_address
> flag into ocotp_params struct, since each platforms has their own, it's easy to indicate this. I tried
> it, and works. >

As long as provider can figure out how the efuse is programmed then it 
is fine with me.


>> 2. In imx devicetree for mac-address nvmem cell make sure you add
>>
>> cell-type = <NVMEM_CELL_TYPE_MAC_ADDRESS>;
>>
>>
>>
>>
>>>
>>>> Option 2: nvmem core handles the post processing.
>>>>
>>>> Pros:
>>>> - provider driver does not need to implement callbacks
>>>>
>>>> Cons:
>>>> - We have to find a way to define vendor specific non-standard
>>>> encoding information in generic bindings which is going to be a
>>>> challenge and high chance of ending up in to much of clutter in generic
>> bindings.
>>>>
>>>> Finally, The way I look at this is that once we start adding
>>>> post-processing in nvmem core then we might endup with code that will
>>>> not be really used for most of the usecases and might endup with
>>>> cases that might not be possible to handle in the core.
>>>>
>>>>
>>>> Does Option 1 work for you?
>>>
>>> Yes, I also prefer to implement it in specific driver, as you mention
>>> above, these code are for very rarely use cases.
>>>
>>> If we chose Option 1, I want to implement it totally in specific
>>> driver(imx-ocotp.c), and I have a draft, could it be acdeptable?
>> Yes, this is the direction, however we need a proper callback to do this. And
>> offset information is still comes from Device tree.
>>
>>
>> Have a look at the patches pushed into topic/post-processing branch.
> 
> I have improved this patch set according above comments and tested it. Also rebase to
> the nvmem/for-next branch.
> 
> I plan to keep you as the nvmem part author and send out this patch set with dts changes. If it's fine for you?

Yes please, can you pick the new patches from the branch before you send 
the series out.

--srini
>    
> Best Regards,
> Joakim Zhang
> 

  reply	other threads:[~2021-09-08  8:49 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-10  7:35 [PATCH V1 0/4] " Joakim Zhang
2021-08-10  7:35 ` [PATCH V1 1/4] bindings: " Joakim Zhang
2021-08-10 15:14   ` Jan Lübbe
2021-08-11  8:05     ` Joakim Zhang
2021-08-11  8:06       ` Joakim Zhang
2021-08-11  9:05         ` Jan Lübbe
2021-08-11 10:16   ` Srinivas Kandagatla
2021-08-17 19:58     ` Rob Herring
2021-08-18  7:54       ` Joakim Zhang
2021-08-24  6:31         ` Joakim Zhang
2021-09-03 12:37         ` Srinivas Kandagatla
2021-09-06  8:58           ` Joakim Zhang
2021-09-07 10:11             ` Srinivas Kandagatla
2021-09-08  7:14               ` Joakim Zhang
2021-09-08  8:48                 ` Srinivas Kandagatla [this message]
2021-09-08  8:57                   ` Joakim Zhang
2021-09-08  9:21                     ` Srinivas Kandagatla
2021-08-10  7:35 ` [PATCH V1 2/4] nvmem: core: introduce "reverse-data" property to reverse buffer Joakim Zhang
2021-08-11  9:52   ` Srinivas Kandagatla
2021-08-11 10:16     ` Joakim Zhang
2021-08-10  7:35 ` [PATCH V1 3/4] arm64: dts: imx8m: add "reverse-data" property for MAC ocopt Joakim Zhang
2021-08-10  7:35 ` [PATCH V1 4/4] arm64: dts: imx8m: remove unused "nvmem_macaddr_swap" property for FEC Joakim Zhang

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=a25379a9-621c-3fda-d933-9cc2d2fc706a@linaro.org \
    --to=srinivas.kandagatla@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=qiangqing.zhang@nxp.com \
    --cc=robh@kernel.org \
    --cc=shawnguo@kernel.org \
    --subject='Re: [PATCH V1 1/4] bindings: nvmem: introduce "reverse-data" property' \
    /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

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).