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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>

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



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux