On 06/16/2016 02:21 PM, Christopher Spinrath wrote:
> Hi Igor,
>
> On 06/16/2016 11:05 AM, Igor Grinberg wrote:
>> Hi Christopher,
>>
>> On 06/15/2016 06:38 PM, Christopher Spinrath wrote:
>>> Hi Nikita,
[...]
>>>> One nit-pick below:
>>>>
>>>>>
>>>>> 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.
>>
> 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!
--
Regards,
Igor.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
