[PATCH] memory: omap-gpmc: fix multi-device handling on the same CS

[PATCH] memory: omap-gpmc: fix multi-device handling on the same CS

[INAGAKI Hiroshi]

This patch fixes the handling of multiple devices on the same CS by
replacing CS name to "name" of node instead of "full_name".

In c2ade654dbf7d02f09ad491f5621fc321d4af96b
("memory: omap-gpmc: Use of_node_name_eq for node name comparisons"),
the name for setting to CS was replaced but it doesn't fit for the
comparison by of_node_name_eq.
In of_node_name_eq, the length for strncmp will be obtained from the
node that trying to register and it doesn't contain the length of
"@<cs>,<offset>". But the base name for comparison passed from
registered CS name contains the prefix, then, that two lengths won't
match and false will be returned, and registration on the same CS
will be failed.

example (Century Systems MA-E350/N, AM3352):

- Device Tree

  /* memory mapped gpio controllers on GPMC */
  gpio@2,2 {
      reg = <2 0x2 0x1>; /* CS2, offset 0x2, IO size 0x1 */
      ...
  };

  gpio@2,10 {
      reg = <2 0x10 0x1>; /* CS2, offset 0x10, IO size 0x1 */
      ...
  };

  gpio@2,12 {
      reg = <2 0x12 0x1>; /* CS2, offset 0x12, IO size 0x1 */
      ...
  };

  gpio@2,14 {
      reg = <2 0x14 0x1>; /* CS2, offset 0x14, IO size 0x1 */
      ...
  };

- dmesg

  [    1.596402] omap-gpmc 50000000.gpmc: cannot request GPMC CS 2
  [    1.596434] omap-gpmc 50000000.gpmc: failed to probe DT child 'gpio': -16
  [    1.596489] omap-gpmc 50000000.gpmc: cannot request GPMC CS 2
  [    1.596511] omap-gpmc 50000000.gpmc: failed to probe DT child 'gpio': -16
  [    1.596564] omap-gpmc 50000000.gpmc: cannot request GPMC CS 2
  [    1.596586] omap-gpmc 50000000.gpmc: failed to probe DT child 'gpio': -16

  ("gpio@2,2" is ok, "gpio@2,10", "gpio@2,12", "gpio@2.14" are fail)

Fixes: c2ade654dbf7d02f09ad491f5621fc321d4af96b
("memory: omap-gpmc: Use of_node_name_eq for node name comparisons")

Signed-off-by: INAGAKI Hiroshi <musashino.open@gmail.com>
---
 drivers/memory/omap-gpmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index d78f73db37c8..3e3e84e34795 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -2202,7 +2202,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
 		dev_err(&pdev->dev, "cannot request GPMC CS %d\n", cs);
 		return ret;
 	}
-	gpmc_cs_set_name(cs, child->full_name);
+	gpmc_cs_set_name(cs, child->name);
 
 	gpmc_read_settings_dt(child, &gpmc_s);
 	gpmc_read_timings_dt(child, &gpmc_t);

base-commit: 13f35b3c72f4075e13a974f439b20b9e26f8f243
-- 
2.25.1

Re: [PATCH] memory: omap-gpmc: fix multi-device handling on the same CS

[Krzysztof Kozlowski]

On 11/01/2023 15:13, INAGAKI Hiroshi wrote:
> This patch fixes the handling of multiple devices on the same CS by

Do not use "This commit/patch".
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> replacing CS name to "name" of node instead of "full_name".
> 
> In c2ade654dbf7d02f09ad491f5621fc321d4af96b
> ("memory: omap-gpmc: Use of_node_name_eq for node name comparisons"),

Use syntax: commit short SHA (".....") as pointed by checkpatch.

> the name for setting to CS was replaced but it doesn't fit for the
> comparison by of_node_name_eq.
> In of_node_name_eq, the length for strncmp will be obtained from the
> node that trying to register and it doesn't contain the length of
> "@<cs>,<offset>". 

Skip explanation what is inside of_node_name_eq() but focus on what the
driver is doing.

> But the base name for comparison passed from
> registered CS name contains the prefix,

What is "the prefix"?

>  then, that two lengths won't
> match and false will be returned, and registration on the same CS
> will be failed.

Unfortunately, based on this, I don't get what is compare with what. I
bet the issue is simple, but based on the description it does not look
like that.

> 
> example (Century Systems MA-E350/N, AM3352):
> 
> - Device Tree
> 
>   /* memory mapped gpio controllers on GPMC */
>   gpio@2,2 {
>       reg = <2 0x2 0x1>; /* CS2, offset 0x2, IO size 0x1 */
>       ...
>   };
> 
>   gpio@2,10 {
>       reg = <2 0x10 0x1>; /* CS2, offset 0x10, IO size 0x1 */
>       ...
>   };
> 
>   gpio@2,12 {
>       reg = <2 0x12 0x1>; /* CS2, offset 0x12, IO size 0x1 */
>       ...
>   };
> 
>   gpio@2,14 {
>       reg = <2 0x14 0x1>; /* CS2, offset 0x14, IO size 0x1 */
>       ...
>   };

Trim it, two entries might be enough to illustrate it.

> 
> - dmesg
> 
>   [    1.596402] omap-gpmc 50000000.gpmc: cannot request GPMC CS 2
>   [    1.596434] omap-gpmc 50000000.gpmc: failed to probe DT child 'gpio': -16
>   [    1.596489] omap-gpmc 50000000.gpmc: cannot request GPMC CS 2
>   [    1.596511] omap-gpmc 50000000.gpmc: failed to probe DT child 'gpio': -16
>   [    1.596564] omap-gpmc 50000000.gpmc: cannot request GPMC CS 2
>   [    1.596586] omap-gpmc 50000000.gpmc: failed to probe DT child 'gpio': -16
> 
>   ("gpio@2,2" is ok, "gpio@2,10", "gpio@2,12", "gpio@2.14" are fail)
> 
> Fixes: c2ade654dbf7d02f09ad491f5621fc321d4af96b
> ("memory: omap-gpmc: Use of_node_name_eq for node name comparisons")

Also not correct tag. Run checkpatch.

> 

No blank lines.

> Signed-off-by: INAGAKI Hiroshi <musashino.open@gmail.com>


> ---
>  drivers/memory/omap-gpmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
> index d78f73db37c8..3e3e84e34795 100644
> --- a/drivers/memory/omap-gpmc.c
> +++ b/drivers/memory/omap-gpmc.c
> @@ -2202,7 +2202,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>  		dev_err(&pdev->dev, "cannot request GPMC CS %d\n", cs);
>  		return ret;
>  	}
> -	gpmc_cs_set_name(cs, child->full_name);
> +	gpmc_cs_set_name(cs, child->name);
>  
>  	gpmc_read_settings_dt(child, &gpmc_s);
>  	gpmc_read_timings_dt(child, &gpmc_t);
> 
> base-commit: 13f35b3c72f4075e13a974f439b20b9e26f8f243

Best regards,
Krzysztof

Re: [PATCH] memory: omap-gpmc: fix multi-device handling on the same CS

[INAGAKI Hiroshi]

Hi Krzysztof,

thank you for your review and sorry for my ugly patch.

On 2023/01/11 23:57, Krzysztof Kozlowski wrote:
> On 11/01/2023 15:13, INAGAKI Hiroshi wrote:
>> This patch fixes the handling of multiple devices on the same CS by
> Do not use "This commit/patch".
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
Okay, I'll improve.

>
>> replacing CS name to "name" of node instead of "full_name".
>>
>> In c2ade654dbf7d02f09ad491f5621fc321d4af96b
>> ("memory: omap-gpmc: Use of_node_name_eq for node name comparisons"),
> Use syntax: commit short SHA (".....") as pointed by checkpatch.
I see, I'll replace.

>
>> the name for setting to CS was replaced but it doesn't fit for the
>> comparison by of_node_name_eq.
>> In of_node_name_eq, the length for strncmp will be obtained from the
>> node that trying to register and it doesn't contain the length of
>> "@<cs>,<offset>".
> Skip explanation what is inside of_node_name_eq() but focus on what the
> driver is doing.
Okay, I'll improve it.

>
>> But the base name for comparison passed from
>> registered CS name contains the prefix,
> What is "the prefix"?
Ahh, it's not a prefix, but suffix...my mistake.

>
>>   then, that two lengths won't
>> match and false will be returned, and registration on the same CS
>> will be failed.
> Unfortunately, based on this, I don't get what is compare with what. I
> bet the issue is simple, but based on the description it does not look
> like that.
Indeed... I wrote it because I felt like I had to explain it in detail, 
but I made it unnecessarily complicated.
I'll improve and make it concise.

>
>> example (Century Systems MA-E350/N, AM3352):
>>
>> - Device Tree
>>
>>    /* memory mapped gpio controllers on GPMC */
>>    gpio@2,2 {
>>        reg = <2 0x2 0x1>; /* CS2, offset 0x2, IO size 0x1 */
>>        ...
>>    };
>>
>>    gpio@2,10 {
>>        reg = <2 0x10 0x1>; /* CS2, offset 0x10, IO size 0x1 */
>>        ...
>>    };
>>
>>    gpio@2,12 {
>>        reg = <2 0x12 0x1>; /* CS2, offset 0x12, IO size 0x1 */
>>        ...
>>    };
>>
>>    gpio@2,14 {
>>        reg = <2 0x14 0x1>; /* CS2, offset 0x14, IO size 0x1 */
>>        ...
>>    };
> Trim it, two entries might be enough to illustrate it.
Okay, I'll reduce.

>
>> - dmesg
>>
>>    [    1.596402] omap-gpmc 50000000.gpmc: cannot request GPMC CS 2
>>    [    1.596434] omap-gpmc 50000000.gpmc: failed to probe DT child 'gpio': -16
>>    [    1.596489] omap-gpmc 50000000.gpmc: cannot request GPMC CS 2
>>    [    1.596511] omap-gpmc 50000000.gpmc: failed to probe DT child 'gpio': -16
>>    [    1.596564] omap-gpmc 50000000.gpmc: cannot request GPMC CS 2
>>    [    1.596586] omap-gpmc 50000000.gpmc: failed to probe DT child 'gpio': -16
>>
>>    ("gpio@2,2" is ok, "gpio@2,10", "gpio@2,12", "gpio@2.14" are fail)
>>
>> Fixes: c2ade654dbf7d02f09ad491f5621fc321d4af96b
>> ("memory: omap-gpmc: Use of_node_name_eq for node name comparisons")
> Also not correct tag. Run checkpatch.
>
> No blank lines.
I'll fix.

>
>> Signed-off-by: INAGAKI Hiroshi <musashino.open@gmail.com>
>
>> ---
>>   drivers/memory/omap-gpmc.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>> index d78f73db37c8..3e3e84e34795 100644
>> --- a/drivers/memory/omap-gpmc.c
>> +++ b/drivers/memory/omap-gpmc.c
>> @@ -2202,7 +2202,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>   		dev_err(&pdev->dev, "cannot request GPMC CS %d\n", cs);
>>   		return ret;
>>   	}
>> -	gpmc_cs_set_name(cs, child->full_name);
>> +	gpmc_cs_set_name(cs, child->name);
>>   
>>   	gpmc_read_settings_dt(child, &gpmc_s);
>>   	gpmc_read_timings_dt(child, &gpmc_t);
>>
>> base-commit: 13f35b3c72f4075e13a974f439b20b9e26f8f243
> Best regards,
> Krzysztof
>

I've completely forgot to run checkpatch.pl before sending...
Before sending the next patch, I will fix the points pointed out this 
time, read the guidelines carefully again and run checkpatch.

Regards,
Hiroshi