Hi Srini,
Sorry for such delay in replies, I am still confused how to
implement it properly, let me please explain the issues
which I faced with:
Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> writes:
> On 16/06/2021 13:33, Vadym Kochan wrote:
>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>> index bca671ff4e54..648373ced6d4 100644
>>>> --- a/drivers/nvmem/core.c
>>>> +++ b/drivers/nvmem/core.c
>>>> @@ -39,6 +39,7 @@ struct nvmem_device {
>>>> nvmem_reg_read_t reg_read;
>>>> nvmem_reg_write_t reg_write;
>>>> struct gpio_desc *wp_gpio;
>>>> + struct nvmem_parser_data *parser_data;
>>> This should be renamed to nvmem_cell_info_parser or something on those lines
>>> to avoid any misunderstanding on what exactly this parser is about.
>>>
>>> May be can totally avoid this by using parser name only during register.
>>>
>> I added this to keep parsed cells particulary for this nvmem in case
>> same parser is used for several nvmem's and mostly because of using also
>> cell lookup info. I will try to also answer your below question why do I need
>> lookups ?
>>
>> I use of_get_mac_address() func to fetch mac-address from nvmem cell.
>> Eventually this func calls of_get_mac_addr_nvmem() which (as I understand it
>> correctly) can find cells via DT by parsing "nvmem-cell-names" or via cell lookup
>> info of platform_device. I use the 2nd option with the following sample
>> solution:
>>
>> ## DT ##
>> eeprom_at24: at24@56 {
>> compatible = "atmel,24c32";
>> nvmem-cell-parser-name = "onie-tlv-cells";
>> reg = <0x56>;
>> };
>>
>> onie_tlv_parser: onie-tlv-cells {
>> compatible = "nvmem-cell-parser";
>> status = "okay";
>>
>> ---> add ability here to map cell con_id to cell_name ?
>>
>> };
>>
>> some_dev_node {
>> compatible = "xxx";
>> base-mac-provider = <&onie_tlv_parser>;
>
> Real nvmem provider is eeprom_at24, why do you use onie_tlv_parse as
> your mac provider?
> If you use eeprom_at24 then of_get_mac_address() should get mac-address
> directly from cell info.
1) This DT node is a trick to register it as a platform_device because of:
static int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr)
{
struct platform_device *pdev = of_find_device_by_node(np);
struct nvmem_cell *cell;
const void *mac;
size_t len;
int ret;
/* Try lookup by device first, there might be a nvmem_cell_lookup
* associated with a given device.
*/
if (pdev) {
ret = nvmem_get_mac_address(&pdev->dev, addr);
put_device(&pdev->dev);
return ret;
}
cell = of_nvmem_cell_get(np, "mac-address");
if (IS_ERR(cell))
return PTR_ERR(cell);
...
}
I tried to use at24_eeprom as ref in DTS file, but this device is not a
platform device but a nvmem bus device, so it fails on:
...
struct platform_device *pdev = of_find_device_by_node(np);
...
/* Try lookup by device first, there might be a nvmem_cell_lookup
* associated with a given device.
*/
if (pdev) {
ret = nvmem_get_mac_address(&pdev->dev, addr);
put_device(&pdev->dev);
return ret;
}
...
Probably this might be fixed by lookup nvmem device too ?
2) Regarding cell lookups registration, I had to use it because
of_nvmem_cell_get() will not find parser cells via OF.
>
>
>> status = "okay";
>> };
>> ########
>>
>> == CODE ==
>> base_mac_np = of_parse_phandle(np, "base-mac-provider", 0);
>> ret = of_get_mac_address(base_mac_np, base_mac);
>> ==========
>>
>>
>> And it works with this implementation because onie-tlv-cells is
>> registered as platform_device which name is the same as parser's name.
>> So the really tricky part for me is to make this cells lookup work.
>
> cell lookups are more of intended for board files, adding them in this
> case is really not correct. The whole purpose of this driver is to
> parse the tlv cell infos into nvmem cell info.
>
>
> --srini
>
>
>>
>> Of course would be great if you can point a way/idea to get rid the need of
>> lookups.
>>
