Skip to contentSkip to conversation

ARM: board: cm-fx6: fix mmc for old revisions of utilite

PATCH New Christopher Spinrath 3 participants 8 messages Jun 8, 2016 – Jun 16, 2016 3 messages not in the archive

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: board: cm-fx6: fix mmc for old revisions of utilite

Old revisions of Utilite (based on cmfx6) do not have a dedicated card detect pin. But the card is removable by the user and card detection can be realized with polling (e.g. supported by Linux).

Add the broken-cd property to the mmc device tree instead of the non-removable property to make card detection possible if polling is supported.

Fixes 41855186afd3 ("arm: mx6: cm-fx6: modify device tree for old revisions of utilite")
---
 board/compulab/cm_fx6/cm_fx6.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
board/compulab/cm_fx6/cm_fx6.c +1 -1
index a21e7b0..712057a 100644
@@ -610,7 +610,7 @@ int ft_board_setup(void *blob, bd_t *bd)
 		fdt_shrink_to_minimum(blob); /* Make room for new properties */
 		nodeoffset = fdt_path_offset(blob, USDHC3_PATH);
 		fdt_delprop(blob, nodeoffset, "cd-gpios");
-		fdt_find_and_setprop(blob, USDHC3_PATH, "non-removable",
+		fdt_find_and_setprop(blob, USDHC3_PATH, "broken-cd",
 				     NULL, 0, 1);
 		fdt_find_and_setprop(blob, USDHC3_PATH, "keep-power-in-suspend",
 				     NULL, 0, 1);
Signature · 1 line
2.8.3
#1 lore raw

PATCH 1/1 ARM: board: cm-fx6: fix mmc for old revisions of utilite

Hi CHristopher,

Quoted 7 lines · Christopher Spinrath

Old revisions of Utilite (based on cmfx6) do not have a dedicated card detect pin. But the card is removable by the user and card detection can be realized with polling (e.g. supported by Linux).

Add the broken-cd property to the mmc device tree instead of the non-removable property to make card detection possible if polling is supported.

One nit-pick below:

Quoted 2 lines
Fixes 41855186afd3 ("arm: mx6: cm-fx6: modify device tree for old revisions of utilite")

This isn't technically a fix; you're enabling new functionality. The original behavior wasn't buggy, it just lacked the card detect feature.

Quoted 8 lines
---
 board/compulab/cm_fx6/cm_fx6.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Signature · 1 line
2.8.3

Regards,
Nikita Kiryanov

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw

PATCH 1/1 ARM: board: cm-fx6: fix mmc for old revisions of utilite

Hi Nikita,

Quoted 7 lines · Nikita Kiryanov

Hi CHristopher,

Quoted 4 lines · Christopher Spinrath

Old revisions of Utilite (based on cmfx6) do not have a dedicated card detect pin. But the card is removable by the user and card detection can be realized with polling (e.g. supported by Linux).

Quoted 5 lines
Quoted 3 lines

Add the broken-cd property to the mmc device tree instead of the non-removable property to make card detection possible if polling is supported.

How is this patch (and, in general, patches for Utilite/cm-fx6) supposed to be merged? Due to get_maintainers your are (the only) maintainer related to the cm-fx6 board. Do you want me to resend the patch (without the Fixes: tag)?

Quoted 8 lines

One nit-pick below:

Quoted 2 lines
Fixes 41855186afd3 ("arm: mx6: cm-fx6: modify device tree for old revisions of utilite")

This isn't technically a fix; you're enabling new functionality. The original behavior wasn't buggy, it just lacked the card detect feature.

Well, the card is clearly removable. So IMHO adding the non-removable property is wrong and this patch corrects/fixes it. But I'm fine either way.

Thanks,
Christopher

Quoted 12 lines
Quoted 8 lines
---
 board/compulab/cm_fx6/cm_fx6.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Signature · 1 line
2.8.3

Regards,
Nikita Kiryanov

#4 lore raw

PATCH 1/1 ARM: board: cm-fx6: fix mmc for old revisions of utilite

Hi Christopher,

Quoted 10 lines · Christopher Spinrath

Hi Nikita,

Quoted 7 lines · Nikita Kiryanov

Hi CHristopher,

Quoted 4 lines · Christopher Spinrath

Old revisions of Utilite (based on cmfx6) do not have a dedicated card detect pin. But the card is removable by the user and card detection can be realized with polling (e.g. supported by Linux).

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

Add the broken-cd property to the mmc device tree instead of the non-removable property to make card detection possible if polling is supported.

How is this patch (and, in general, patches for Utilite/cm-fx6) supposed to be merged?

Well, you've done this almost correctly. You just need to add Stefano Babic <[email protected]>
(who is the maintainer of imx) to "cc" or "to" list. Added now.

Quoted 2 lines

Due to get_maintainers your are (the only) maintainer related to the cm-fx6 board.

That is a separate discussion (e.g. how boards of a specific vendor, but different architectures/platforms should be listed in the MAINTAINERS files).

Quoted 6 lines

Do you want me to resend the patch (without
the Fixes: tag)?

Quoted 3 lines

One nit-pick below:

Quoted 1 line
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Fixes 41855186afd3 ("arm: mx6: cm-fx6: modify device tree for old revisions of utilite")

This isn't technically a fix; you're enabling new functionality. The original behavior wasn't buggy, it just lacked the card detect feature.

Well, the card is clearly removable. So IMHO adding the non-removable property is wrong and this patch corrects/fixes it. But I'm fine either way.

Just a little explanation...
Mechanically the card _is_ removable, but for revisions < 1.3, it will result in errors on the bus as no removal event will be sent to the subsystem. Moreover, if I'm not mistaken, you have the PRO model, right? In PRO model, you have internal storage (e.g. SSD) which makes the SD card an additional and sensibly removable device...
There are additional Utilite models which have the SD card as the only storage device and those models have the rootfs on the SD card. In such case, IMO, it is much more appropriate to state that it should be non-removable.

Signature · 2 lines
Regards,
Igor.
#5 lore raw

PATCH 1/1 ARM: board: cm-fx6: fix mmc for old revisions of utilite

In reply to Christopher Spinrath

Hi Christopher,

Quoted 24 lines · Christopher Spinrath

Hi Nikita,

Quoted 7 lines · Nikita Kiryanov

Hi CHristopher,

Quoted 4 lines · Christopher Spinrath

Old revisions of Utilite (based on cmfx6) do not have a dedicated card detect pin. But the card is removable by the user and card detection can be realized with polling (e.g. supported by Linux).

Quoted 5 lines
Quoted 3 lines

Add the broken-cd property to the mmc device tree instead of the non-removable property to make card detection possible if polling is supported.

How is this patch (and, in general, patches for Utilite/cm-fx6) supposed to be merged? Due to get_maintainers your are (the only) maintainer related to the cm-fx6 board. Do you want me to resend the patch (without the Fixes: tag)?

Quoted 3 lines

One nit-pick below:

Quoted 1 line
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Fixes 41855186afd3 ("arm: mx6: cm-fx6: modify device tree for old revisions of utilite")

This isn't technically a fix; you're enabling new functionality. The original behavior wasn't buggy, it just lacked the card detect feature.

Well, the card is clearly removable. So IMHO adding the non-removable property is wrong and this patch corrects/fixes it. But I'm fine either way.

I prefer it without the "Fixes" line, because to me it implies a bug fix whereas the behavior in the linked patch was an intentional compromise.

Regards,
Nikita Kiryanov

A message that is not in the archive (sent off-list or not yet received): [email protected]

#7 lore raw

PATCH 1/1 ARM: board: cm-fx6: fix mmc for old revisions of utilite

Hi Igor,

Quoted 19 lines · Igor Grinberg

Hi Christopher,

Quoted 16 lines · Christopher Spinrath

Hi Nikita,

Quoted 12 lines · Nikita Kiryanov

Hi CHristopher,

Quoted 7 lines · Christopher Spinrath

Old revisions of Utilite (based on cmfx6) do not have a dedicated card detect pin. But the card is removable by the user and card detection can be realized with polling (e.g. supported by Linux).

Add the broken-cd property to the mmc device tree instead of the non-removable property to make card detection possible if polling is supported.

Quoted 8 lines
Quoted 2 lines

How is this patch (and, in general, patches for Utilite/cm-fx6) supposed to be merged?

Well, you've done this almost correctly. You just need to add Stefano Babic <[email protected]>
(who is the maintainer of imx) to "cc" or "to" list. Added now.

Thanks!

Quoted 6 lines
Quoted 2 lines

Due to get_maintainers your are (the only) maintainer related to the cm-fx6 board.

That is a separate discussion (e.g. how boards of a specific vendor, but different architectures/platforms should be listed in the MAINTAINERS files).

I expected the output of get_maintainers to be complete. But ok, now I know it's not.

Quoted 14 lines
Quoted 6 lines

Do you want me to resend the patch (without
the Fixes: tag)?

Quoted 3 lines

One nit-pick below:

Quoted 1 line
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Fixes 41855186afd3 ("arm: mx6: cm-fx6: modify device tree for old revisions of utilite")

This isn't technically a fix; you're enabling new functionality. The original behavior wasn't buggy, it just lacked the card detect feature.

Well, the card is clearly removable. So IMHO adding the non-removable property is wrong and this patch corrects/fixes it. But I'm fine either way.

Quoted 11 lines

Just a little explanation...
Mechanically the card _is_ removable, but for revisions < 1.3, it will result in errors on the bus as no removal event will be sent to the subsystem. Moreover, if I'm not mistaken, you have the PRO model, right? In PRO model, you have internal storage (e.g. SSD) which makes the SD card an additional and sensibly removable device...
There are additional Utilite models which have the SD card as the only storage device and those models have the rootfs on the SD card. In such case, IMO, it is much more appropriate to state that it should be non-removable.

With the broken-cd property the driver/subsystem knows that the card may have been removed and checks that if an (false positive) error occurs.

Indeed, I have the Pro model but even for the other models there are use cases where the card may be removed. For instance, you can use netboot (think of thin clients) or boot from usb storage. So IMO the broken-cd property is a better choice for all models.

Cheers,
Christopher

#8 lore raw

PATCH 1/1 ARM: board: cm-fx6: fix mmc for old revisions of utilite

Quoted 7 lines · Christopher Spinrath

Hi Igor,

Quoted 4 lines · Igor Grinberg

Hi Christopher,

Quoted 1 line · Christopher Spinrath

Hi Nikita,

[...]

Quoted 22 lines
Quoted 11 lines
Quoted 10 lines
Quoted 8 lines

One nit-pick below:

Quoted 2 lines
Fixes 41855186afd3 ("arm: mx6: cm-fx6: modify device tree for old revisions of utilite")

This isn't technically a fix; you're enabling new functionality. The original behavior wasn't buggy, it just lacked the card detect feature.

Well, the card is clearly removable. So IMHO adding the non-removable property is wrong and this patch corrects/fixes it. But I'm fine either way.

Quoted 11 lines

Just a little explanation...
Mechanically the card _is_ removable, but for revisions < 1.3, it will result in errors on the bus as no removal event will be sent to the subsystem. Moreover, if I'm not mistaken, you have the PRO model, right? In PRO model, you have internal storage (e.g. SSD) which makes the SD card an additional and sensibly removable device...
There are additional Utilite models which have the SD card as the only storage device and those models have the rootfs on the SD card. In such case, IMO, it is much more appropriate to state that it should be non-removable.

Quoted 7 lines

With the broken-cd property the driver/subsystem knows that the card may have been removed and checks that if an (false positive) error occurs.

Indeed, I have the Pro model but even for the other models there are use cases where the card may be removed. For instance, you can use netboot (think of thin clients) or boot from usb storage. So IMO the broken-cd property is a better choice for all models.

Yeah, no problem - I'm not saying we should keep the original, just explaining the rationale behind what was done and why we do not see it as a fix, but rather as an improvement to cover more use cases.

Thanks for the patch!

Signature · 2 lines
Regards,
Igor.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#10 lore raw

PATCH 1/1 ARM: board: cm-fx6: fix mmc for old revisions of utilite

Hi Nikita,

Quoted 29 lines · Nikita Kiryanov

Hi Christopher,

Quoted 26 lines · Christopher Spinrath

Hi Nikita,

Quoted 7 lines · Nikita Kiryanov

Hi CHristopher,

Quoted 4 lines · Christopher Spinrath

Old revisions of Utilite (based on cmfx6) do not have a dedicated card detect pin. But the card is removable by the user and card detection can be realized with polling (e.g. supported by Linux).

Quoted 5 lines
Quoted 3 lines

Add the broken-cd property to the mmc device tree instead of the non-removable property to make card detection possible if polling is supported.

How is this patch (and, in general, patches for Utilite/cm-fx6) supposed to be merged? Due to get_maintainers your are (the only) maintainer related to the cm-fx6 board. Do you want me to resend the patch (without the Fixes: tag)?

Quoted 5 lines

One nit-pick below:

Quoted 2 lines
Fixes 41855186afd3 ("arm: mx6: cm-fx6: modify device tree for old revisions of utilite")
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines

This isn't technically a fix; you're enabling new functionality. The original behavior wasn't buggy, it just lacked the card detect feature.

Well, the card is clearly removable. So IMHO adding the non-removable property is wrong and this patch corrects/fixes it. But I'm fine either way.

I prefer it without the "Fixes" line, because to me it implies a bug fix whereas the behavior in the linked patch was an intentional compromise.

Ok, I will send a v2 without the fixes tag and with your Acked-By.

Thanks,
Christopher

Quoted 3 lines

Regards,
Nikita Kiryanov