Re: [PATCH V1 1/4] bindings: nvmem: introduce "reverse-data" property
From: Srinivas Kandagatla
Date: Wed Sep 08 2021 - 05:21:59 EST
On 08/09/2021 09:57, Joakim Zhang wrote:
Hi Srinivas,
-----Original Message-----
From: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
Sent: 2021年9月8日 16:49
To: Joakim Zhang <qiangqing.zhang@xxxxxxx>; Rob Herring
<robh@xxxxxxxxxx>
Cc: shawnguo@xxxxxxxxxx; kernel@xxxxxxxxxxxxxx; dl-linux-imx
<linux-imx@xxxxxxx>; devicetree@xxxxxxxxxxxxxxx;
linux-kernel@xxxxxxxxxxxxxxx
Subject: Re: [PATCH V1 1/4] bindings: nvmem: introduce "reverse-data"
property
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%2F&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cb8b85
eab6bc34
917b86e08d972a57dee%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
7C6376
66877370588296%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC
JQIjoiV2
luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=diFgK2ufOUK
eXwd
0Ez8pCFjCUH8rXz5jfW7io8KDKmw%3D&reserved=0
el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fsrini%2Fnvmem.git%2Flog%
2F%3Fh%3Dtopic%2Fpost-processing&data=04%7C01%7Cqiangqing.zhan
g%40nxp.com%7Cadfa3ba63c634937876308d971e7e71f%7C686ea1d3bc2b4c6
fa92cd99c5c301635%7C0%7C0%7C637666063097239185%7CUnknown%7CT
WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJ
XVCI6Mn0%3D%7C1000&sdata=W9yAnGm9rYzlSZuAAGiN4VHUtKYUTt9S
oyGQ9QsY7fI%3D&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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern
el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fsrini%2Fnvmem.git%2Fcom
mit%2F%3Fh%3Dtopic%2Fpost-processing%26id%3D624f2cc99b48bbfe05c11e
58fb73f84abb1a646e&data=04%7C01%7Cqiangqing.zhang%40nxp.com%
7Cb8b85eab6bc34917b86e08d972a57dee%7C686ea1d3bc2b4c6fa92cd99c5c3
01635%7C0%7C0%7C637666877370598253%7CUnknown%7CTWFpbGZsb3d8e
yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
%7C1000&sdata=APDzSbLob%2FRiZyyhU7VAhoUEAmSG95NsilQDQ53Hbf
A%3D&reserved=0
of_get_property() can't get the cell value, so I change to
of_property_read_s32()
2)
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern
el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fsrini%2Fnvmem.git%2Fcom
mit%2F%3Fh%3Dtopic%2Fpost-processing%26id%3Da424302c7b15da41e1e8d
e56b0c78021b9a96c1e&data=04%7C01%7Cqiangqing.zhang%40nxp.com
%7Cb8b85eab6bc34917b86e08d972a57dee%7C686ea1d3bc2b4c6fa92cd99c5c
301635%7C0%7C0%7C637666877370598253%7CUnknown%7CTWFpbGZsb3d8
eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
%7C1000&sdata=5E49DVzkpBVdkA4a%2B9tMXN%2B6k%2BG%2B3rQuVJ
qTUgdbmKU%3D&reserved=0
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.
As you define the type variable is "int", so had better use of_property_read_s32(), instead if of_property_read_u32(), right?
We should probably make that u32, as we are not expecting any negative
range.
I tried to fix this in new patches.
--srini
Best Regards,
Joakim Zhang