@@ -61,6 +61,11 @@ patternProperties:description:Size in bit within the address range specified by reg.+reverse-data:+$ref:/schemas/types.yaml#/definitions/flag+description:+Reverse the data that read from the storage device.+required:-reg
This patch intends to introduce "reverse-data" property at nvmem
provider side to reverse buffer.
For a case used from Ethernet driver, of_get_mac_address() may call
of_get_mac_addr_nvmem() to get MAC address from nvmem device. MAC address
programed in imx-ocopt is MSB first in lowest address, so need reverse
data then feedback to nvmem consumer. E.g. MAC address read from
imx-ocopt is 98:e2:06:9f:04:00, after reversed the data is
00:04:9f:06:e2:98.
Signed-off-by: Joakim Zhang <redacted>
---
drivers/nvmem/core.c | 30 ++++++++++++++++++++++++++++++
include/linux/nvmem-provider.h | 2 ++
2 files changed, 32 insertions(+)
Remove unused "nvmem_macaddr_swap" property for FEC, there is no info in both
dt-binding and driver, so it's safe to remove it.
Signed-off-by: Joakim Zhang <redacted>
---
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 1 -
arch/arm64/boot/dts/freescale/imx8mn.dtsi | 1 -
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 1 -
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 1 -
4 files changed, 4 deletions(-)
@@ -61,6 +61,11 @@ patternProperties:description:Size in bit within the address range specified by reg.+reverse-data:+$ref:/schemas/types.yaml#/definitions/flag+description:+Reverse the data that read from the storage device.
I'd prefer if it was more explicit that the *bytes* will be reversed. Otherwise
a reader might think that this is reversing on the *bit* level.
Jan
@@ -61,6 +61,11 @@ patternProperties:description:Size in bit within the address range specified by reg.+reverse-data:+$ref:/schemas/types.yaml#/definitions/flag+description:+Reverse the data that read from the storage device.
I'd prefer if it was more explicit that the *bytes* will be reversed. Otherwise a
reader might think that this is reversing on the *bit* level.
Make sense, how about 'reverse-byte-order'?
Best Regards,
Joakim Zhang
@@ -61,6 +61,11 @@ patternProperties:description:Size in bit within the address range specified by reg.+reverse-data:+$ref:/schemas/types.yaml#/definitions/flag+description:+Reverse the data that read from the storage device.
I'd prefer if it was more explicit that the *bytes* will be reversed.
Otherwise a reader might think that this is reversing on the *bit* level.
Make sense, how about 'reverse-byte-order'?
Sorry, 'reverse-bytes-order'.
Best Regards,
Joakim Zhang
@@ -61,6 +61,11 @@ patternProperties:description:Size in bit within the address range specified by reg.+reverse-data:+$ref:/schemas/types.yaml#/definitions/flag+description:+Reverse the data that read from the storage device.
I'd prefer if it was more explicit that the *bytes* will be reversed.
Otherwise a reader might think that this is reversing on the *bit* level.
Make sense, how about 'reverse-byte-order'?
Sorry, 'reverse-bytes-order'.
'reverse-byte-order' sounds better to me, but I'm not a native English speaker.
I'd be fine with either, though.
Thanks
Jan
This patch intends to introduce "reverse-data" property at nvmem
provider side to reverse buffer.
For a case used from Ethernet driver, of_get_mac_address() may call
of_get_mac_addr_nvmem() to get MAC address from nvmem device. MAC address
programed in imx-ocopt is MSB first in lowest address, so need reverse
Is all the data stored in imx-ocopt reverse byte order?
or someone decided to change the order for only mac-address?
quoted hunk
data then feedback to nvmem consumer. E.g. MAC address read from
imx-ocopt is 98:e2:06:9f:04:00, after reversed the data is
00:04:9f:06:e2:98.
Signed-off-by: Joakim Zhang <redacted>
---
drivers/nvmem/core.c | 30 ++++++++++++++++++++++++++++++
include/linux/nvmem-provider.h | 2 ++
2 files changed, 32 insertions(+)
This flag also needs to go in struct nvmem_cell_info too.
quoted hunk
};
static DEFINE_MUTEX(nvmem_mutex);
@@ -92,6 +93,26 @@ static int __nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset, return -EINVAL; }+static int nvmem_buffer_reverse(void *bufaddr, int len)+{+ u8 *buf = (u8 *)bufaddr;+ u8 *temp;+ int i;++ temp = kzalloc(len, GFP_KERNEL);+ if (!temp)+ return -ENOMEM;++ memcpy(temp, buf, len);++ for (i = 0; i < len; i++)+ buf[i] = temp[len - i - 1];+
You could possibly simplify this.
static void nvmem_buffer_reverse(u8 *buf, int len)
{
int i;
for (i = 0; i < len/2; i++)
swap(buf[i], buf[len - i - 1]);
}
quoted hunk
+ kfree(temp);++ return 0;+}+ static int nvmem_access_with_keepouts(struct nvmem_device *nvmem, unsigned int offset, void *val, size_t bytes, int write)
@@ -705,6 +726,9 @@ static int nvmem_add_cells_from_of(struct nvmem_device *nvmem) cell->bytes = be32_to_cpup(addr); cell->name = kasprintf(GFP_KERNEL, "%pOFn", child);+ if (of_property_read_bool(child, "reverse-data"))+ cell->flags |= NVMEM_FLAGS_REVERSE_DATA;+ addr = of_get_property(child, "bits", &len); if (addr && len == (2 * sizeof(u32))) { cell->bit_offset = be32_to_cpup(addr++);
@@ -1398,6 +1422,12 @@ static int __nvmem_cell_read(struct nvmem_device *nvmem, if (cell->bit_offset || cell->nbits) nvmem_shift_read_buffer_in_place(cell, buf);+ if (cell->flags & NVMEM_FLAGS_REVERSE_DATA) {+ rc = nvmem_buffer_reverse(buf, cell->bytes);+ if (rc < 0)+ return rc;+ }+ if (len) *len = cell->bytes;
This patch intends to introduce "reverse-data" property at nvmem
provider side to reverse buffer.
For a case used from Ethernet driver, of_get_mac_address() may call
of_get_mac_addr_nvmem() to get MAC address from nvmem device. MAC
address programed in imx-ocopt is MSB first in lowest address, so need
reverse
Is all the data stored in imx-ocopt reverse byte order?
No, it depends on how users program these data.
or someone decided to change the order for only mac-address?
No, will not change the order for mac-address, would be consistent.
quoted
data then feedback to nvmem consumer. E.g. MAC address read from
imx-ocopt is 98:e2:06:9f:04:00, after reversed the data is
00:04:9f:06:e2:98.
Signed-off-by: Joakim Zhang <redacted>
---
drivers/nvmem/core.c | 30
This flag also needs to go in struct nvmem_cell_info too.
OK.
quoted
};
static DEFINE_MUTEX(nvmem_mutex);
@@ -92,6 +93,26 @@ static int __nvmem_reg_write(struct nvmem_device
*nvmem, unsigned int offset,
quoted
return -EINVAL; }+static int nvmem_buffer_reverse(void *bufaddr, int len) {+ u8 *buf = (u8 *)bufaddr;+ u8 *temp;+ int i;++ temp = kzalloc(len, GFP_KERNEL);+ if (!temp)+ return -ENOMEM;++ memcpy(temp, buf, len);++ for (i = 0; i < len; i++)+ buf[i] = temp[len - i - 1];+
You could possibly simplify this.
static void nvmem_buffer_reverse(u8 *buf, int len) {
int i;
for (i = 0; i < len/2; i++)
swap(buf[i], buf[len - i - 1]);
}
Thanks.
quoted
+ kfree(temp);++ return 0;+}+ static int nvmem_access_with_keepouts(struct nvmem_device *nvmem, unsigned int offset, void *val, size_t bytes, int write)
@@ -705,6 +726,9 @@ static int nvmem_add_cells_from_of(struct
@@ -61,6 +61,11 @@ patternProperties:description:Size in bit within the address range specified by reg.+reverse-data:+$ref:/schemas/types.yaml#/definitions/flag+description:+Reverse the data that read from the storage device.+
This new property is only going to solve one of the reverse order issue here.
If I remember correctly we have mac-address stored in various formats ex: from old thread I can see
Type 1: Octets in ASCII without delimiters. (Swapped/non-Swapped)
Type 2: Octets in ASCII with delimiters like (":", ",", ".", "-"... so on) (Swapped/non-Swapped)
Type 3: Is the one which stores mac address in Type1/2 but this has to be incremented to be used on other instances of eth.
Type 4: Octets as bytes/u8, swapped/non-swapped
I think its right time to consider adding compatibles to nvmem-cells to be able to specify encoding information and handle post processing.
Lets see what Rob would say on this approach.
--srini
@@ -61,6 +61,11 @@ patternProperties:description:Size in bit within the address range specified by reg.+reverse-data:+$ref:/schemas/types.yaml#/definitions/flag+description:+Reverse the data that read from the storage device.+
This new property is only going to solve one of the reverse order issue
here.
If I remember correctly we have mac-address stored in various formats ex:
from old thread I can see
Type 1: Octets in ASCII without delimiters. (Swapped/non-Swapped)
Type 2: Octets in ASCII with delimiters like (":", ",", ".", "-"... so on)
(Swapped/non-Swapped)
Type 3: Is the one which stores mac address in Type1/2 but this has to be
incremented to be used on other instances of eth.
Type 4: Octets as bytes/u8, swapped/non-swapped
I think its right time to consider adding compatibles to nvmem-cells to be
able to specify encoding information and handle post processing.
Yes. Trying to handle this with never ending new properties will end up
with a mess. At some point, you just need code to parse the data.
Rob
@@ -61,6 +61,11 @@ patternProperties:description:Size in bit within the address range specified by reg.+reverse-data:+$ref:/schemas/types.yaml#/definitions/flag+description:+Reverse the data that read from the storage device.+
This new property is only going to solve one of the reverse order
issue here.
If I remember correctly we have mac-address stored in various formats ex:
from old thread I can see
Type 1: Octets in ASCII without delimiters. (Swapped/non-Swapped) Type
2: Octets in ASCII with delimiters like (":", ",", ".", "-"... so on)
(Swapped/non-Swapped)
Type 3: Is the one which stores mac address in Type1/2 but this has to
be incremented to be used on other instances of eth.
Type 4: Octets as bytes/u8, swapped/non-swapped
I think its right time to consider adding compatibles to nvmem-cells
to be able to specify encoding information and handle post processing.
Yes. Trying to handle this with never ending new properties will end up with a
mess. At some point, you just need code to parse the data.
Thanks, Rob.
Hi Srinivas,
Do you plan to implement it?
Or need me follow up? If yes, please input your insights to point me how to work for it.
Thanks.
Best Regards,
Joakim Zhang
@@ -61,6 +61,11 @@ patternProperties:description:Size in bit within the address range specified by
reg.
quoted
quoted
quoted
+ reverse-data:+ $ref: /schemas/types.yaml#/definitions/flag+ description:+ Reverse the data that read from the storage device.+
This new property is only going to solve one of the reverse order
issue here.
If I remember correctly we have mac-address stored in various formats ex:
from old thread I can see
Type 1: Octets in ASCII without delimiters. (Swapped/non-Swapped)
Type
2: Octets in ASCII with delimiters like (":", ",", ".", "-"... so
on)
(Swapped/non-Swapped)
Type 3: Is the one which stores mac address in Type1/2 but this has
to be incremented to be used on other instances of eth.
Type 4: Octets as bytes/u8, swapped/non-swapped
I think its right time to consider adding compatibles to nvmem-cells
to be able to specify encoding information and handle post processing.
Yes. Trying to handle this with never ending new properties will end
up with a mess. At some point, you just need code to parse the data.
Thanks, Rob.
Hi Srinivas,
Do you plan to implement it?
Or need me follow up? If yes, please input your insights to point me how to
work for it.
@@ -61,6 +61,11 @@ patternProperties:description:Size in bit within the address range specified by reg.+reverse-data:+$ref:/schemas/types.yaml#/definitions/flag+description:+Reverse the data that read from the storage device.+
This new property is only going to solve one of the reverse order
issue here.
If I remember correctly we have mac-address stored in various formats ex:
from old thread I can see
Type 1: Octets in ASCII without delimiters. (Swapped/non-Swapped) Type
2: Octets in ASCII with delimiters like (":", ",", ".", "-"... so on)
(Swapped/non-Swapped)
Type 3: Is the one which stores mac address in Type1/2 but this has to
be incremented to be used on other instances of eth.
Type 4: Octets as bytes/u8, swapped/non-swapped
I think its right time to consider adding compatibles to nvmem-cells
to be able to specify encoding information and handle post processing.
Yes. Trying to handle this with never ending new properties will end up with a
mess. At some point, you just need code to parse the data.
Thanks, Rob.
Hi Srinivas,
Firstly Sorry for taking so long to reply as I was on vacation.
Do you plan to implement it?
No, Am not planning to do this. But am happy to walk-thru the ideas that I have.
Or need me follow up? If yes, please input your insights to point me how to work for it.
Do we have some kind of meta data/information in nvmem memory to indicate the storage encoding?
Am I correct to say that this is only issue with mac-address nvmem cell?
Irrespective of where this encoding info comes from we have 2 options.
Option 1: Add callback to handle mac-address post-processing with in the provider driver.
Pros:
- It can deal with vendor specific non-standard encodings, and code is mostly with-in vendor specific nvmem provider driver and bindings.
- will keep nvmem core simple w.r.t handling data.
Cons:
- provider driver implement callback and new bindings.
- might need to add a nvmem-cell-type binding to be able differentiate the cell types and handle post-processing.
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?
--srini
@@ -61,6 +61,11 @@ patternProperties:description:Size in bit within the address range specified by
reg.
quoted
quoted
quoted
quoted
+ reverse-data:+ $ref: /schemas/types.yaml#/definitions/flag+ description:+ Reverse the data that read from the storage device.+
This new property is only going to solve one of the reverse order
issue here.
If I remember correctly we have mac-address stored in various formats ex:
from old thread I can see
Type 1: Octets in ASCII without delimiters. (Swapped/non-Swapped)
Type
2: Octets in ASCII with delimiters like (":", ",", ".", "-"... so
on)
(Swapped/non-Swapped)
Type 3: Is the one which stores mac address in Type1/2 but this has
to be incremented to be used on other instances of eth.
Type 4: Octets as bytes/u8, swapped/non-swapped
I think its right time to consider adding compatibles to nvmem-cells
to be able to specify encoding information and handle post processing.
Yes. Trying to handle this with never ending new properties will end
up with a mess. At some point, you just need code to parse the data.
Thanks, Rob.
Hi Srinivas,
Firstly Sorry for taking so long to reply as I was on vacation.
quoted
Do you plan to implement it?
No, Am not planning to do this. But am happy to walk-thru the ideas that I
have.
quoted
Or need me follow up? If yes, please input your insights to point me how to
work for it.
Do we have some kind of meta data/information in nvmem memory to indicate
the storage encoding?
No, none of these.
Am I correct to say that this is only issue with mac-address nvmem cell?
I think, yes.
Irrespective of where this encoding info comes from we have 2 options.
Option 1: Add callback to handle mac-address post-processing with in the
provider driver.
Sorry, I am not very familiar with nvmem framework, what's this "callback" mean?
Do you also want to introduce a common callback for different vendor drivers to
work for mac-address post-processing? Extend the " struct nvmem_config"?
Pros:
- It can deal with vendor specific non-standard encodings, and code is mostly
with-in vendor specific nvmem provider driver and bindings.
- will keep nvmem core simple w.r.t handling data.
Cons:
- provider driver implement callback and new bindings.
- might need to add a nvmem-cell-type binding to be able differentiate the cell
types and handle post-processing.
Ahhh, I am not quite understand how to implement for it? Could you please give some
draft hints?
If we extend the struct nvmem_config, add a callback to handle mac address, how can we
determine which is the mac-address device node? There is no device node info from .reg_read
callback.
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?
@@ -76,6 +76,8 @@#define IMX_OCOTP_WR_UNLOCK 0x3E770000#define IMX_OCOTP_READ_LOCKED_VAL 0xBADABADA+#define IMX_OCOTP_MAC_MAX 0x2 /* The maximum numbers of MAC instance */+staticDEFINE_MUTEX(ocotp_mutex);structocotp_priv{
@@ -211,6 +220,20 @@ static int imx_ocotp_read(void *context, unsigned int offset,}index=offset%4;++/* Handle MAC address reverse byte if required */+for(i=0;i<IMX_OCOTP_MAC_MAX;i++){+if(offset==priv->params->mac[i].offset&&+bytes==priv->params->mac[i].size&&+priv->params->mac[i].reverse_byte){+u8*org=&p[index];+intj;++for(j=0;j<bytes/2;j++)+swap(org[j],org[bytes-j-1]);+}+}+memcpy(val,&p[index],bytes);read_end:
@@ -61,6 +61,11 @@ patternProperties:description:Size in bit within the address range specified by
reg.
quoted
quoted
quoted
quoted
+ reverse-data:+ $ref: /schemas/types.yaml#/definitions/flag+ description:+ Reverse the data that read from the storage device.+
This new property is only going to solve one of the reverse order
issue here.
If I remember correctly we have mac-address stored in various formats ex:
from old thread I can see
Type 1: Octets in ASCII without delimiters. (Swapped/non-Swapped)
Type
2: Octets in ASCII with delimiters like (":", ",", ".", "-"... so
on)
(Swapped/non-Swapped)
Type 3: Is the one which stores mac address in Type1/2 but this has
to be incremented to be used on other instances of eth.
Type 4: Octets as bytes/u8, swapped/non-swapped
I think its right time to consider adding compatibles to nvmem-cells
to be able to specify encoding information and handle post processing.
Yes. Trying to handle this with never ending new properties will end
up with a mess. At some point, you just need code to parse the data.
Thanks, Rob.
Hi Srinivas,
Firstly Sorry for taking so long to reply as I was on vacation.
quoted
Do you plan to implement it?
No, Am not planning to do this. But am happy to walk-thru the ideas that I
have.
quoted
Or need me follow up? If yes, please input your insights to point me how to
work for it.
Do we have some kind of meta data/information in nvmem memory to indicate
the storage encoding?
No, none of these.
quoted
Am I correct to say that this is only issue with mac-address nvmem cell?
I think, yes.
quoted
Irrespective of where this encoding info comes from we have 2 options.
Option 1: Add callback to handle mac-address post-processing with in the
provider driver.
Sorry, I am not very familiar with nvmem framework, what's this "callback" mean?
Do you also want to introduce a common callback for different vendor drivers to
work for mac-address post-processing? Extend the " struct nvmem_config"?
quoted
Pros:
- It can deal with vendor specific non-standard encodings, and code is mostly
with-in vendor specific nvmem provider driver and bindings.
- will keep nvmem core simple w.r.t handling data.
Cons:
- provider driver implement callback and new bindings.
- might need to add a nvmem-cell-type binding to be able differentiate the cell
types and handle post-processing.
Ahhh, I am not quite understand how to implement for it? Could you please give some
draft hints?
If we extend the struct nvmem_config, add a callback to handle mac address, how can we
determine which is the mac-address device node? There is no device node info from .reg_read
callback.
I have pushed some nvmem core patches which are just compile tested to https://git.kernel.org/pub/scm/linux/kernel/git/srini/nvmem.git/log/?h=topic/post-processing
This should provide the callback hook I was talking about.
Can you take a look at them and let me know if it works for you.
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.
2. In imx devicetree for mac-address nvmem cell make sure you add
cell-type = <NVMEM_CELL_TYPE_MAC_ADDRESS>;
quoted
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.
--srini
@@ -76,6 +76,8 @@#define IMX_OCOTP_WR_UNLOCK 0x3E770000#define IMX_OCOTP_READ_LOCKED_VAL 0xBADABADA+#define IMX_OCOTP_MAC_MAX 0x2 /* The maximum numbers of MAC instance */+staticDEFINE_MUTEX(ocotp_mutex);structocotp_priv{
@@ -211,6 +220,20 @@ static int imx_ocotp_read(void *context, unsigned int offset,}index=offset%4;++/* Handle MAC address reverse byte if required */+for(i=0;i<IMX_OCOTP_MAC_MAX;i++){+if(offset==priv->params->mac[i].offset&&+bytes==priv->params->mac[i].size&&+priv->params->mac[i].reverse_byte){+u8*org=&p[index];+intj;++for(j=0;j<bytes/2;j++)+swap(org[j],org[bytes-j-1]);+}+}+memcpy(val,&p[index],bytes);read_end:
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&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.
Can you take a look at them and let me know if it works for you.
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.
2. In imx devicetree for mac-address nvmem cell make sure you add
cell-type = <NVMEM_CELL_TYPE_MAC_ADDRESS>;
quoted
quoted
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.
quoted
quoted
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?
Best Regards,
Joakim Zhang
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&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,
quoted
Can you take a look at them and let me know if it works for you.
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.
quoted
2. In imx devicetree for mac-address nvmem cell make sure you add
cell-type = <NVMEM_CELL_TYPE_MAC_ADDRESS>;
quoted
quoted
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.
quoted
quoted
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
if (!nvmem->cell_post_process) {} should be if (nvmem->cell_post_process)
{}, if we have this callback, we need do the post-processing.
quoted
I have pushed these changes now to the branch.
quoted
quoted
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.
quoted
quoted
2. In imx devicetree for mac-address nvmem cell make sure you add
cell-type = <NVMEM_CELL_TYPE_MAC_ADDRESS>;
quoted
quoted
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.
quoted
quoted
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
quoted
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?
Best Regards,
Joakim Zhang
if (!nvmem->cell_post_process) {} should be if (nvmem->cell_post_process)
{}, if we have this callback, we need do the post-processing.
quoted
I have pushed these changes now to the branch.
quoted
quoted
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.
quoted
quoted
2. In imx devicetree for mac-address nvmem cell make sure you add
cell-type = <NVMEM_CELL_TYPE_MAC_ADDRESS>;
quoted
quoted
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.
quoted
quoted
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
quoted
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