All of lore.kernel.org
 help / color / mirror / Atom feed
From: Igor Grinberg <grinberg@compulab.co.il>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] ARM: board: cm-fx6: fix mmc for old revisions of utilite
Date: Thu, 16 Jun 2016 15:46:54 +0300	[thread overview]
Message-ID: <57629FBE.7030001@compulab.co.il> (raw)
In-Reply-To: <de60db96b3a6422288bb71f0b0829bd1@rwthex-s1-b.rwth-ad.de>

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.

  reply	other threads:[~2016-06-16 12:46 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-08 19:02 [U-Boot] [PATCH] ARM: board: cm-fx6: fix mmc for old revisions of utilite Christopher Spinrath
2016-06-15 15:15 ` Nikita Kiryanov
     [not found] ` <d14689fedcc6431eb290feab11572a81@rwthex-w2-b.rwth-ad.de>
2016-06-15 15:38   ` Christopher Spinrath
2016-06-16  9:05     ` Igor Grinberg
2016-06-16 10:40     ` Nikita Kiryanov
     [not found]     ` <8541bdd129d345eda74f0bdb0444d2fc@rwthex-w2-a.rwth-ad.de>
2016-06-16 11:21       ` Christopher Spinrath
2016-06-16 12:46         ` Igor Grinberg [this message]
     [not found]     ` <f56e4e04d9ab4fb9bc974570a103a0d0@rwthex-w2-b.rwth-ad.de>
2016-06-16 11:22       ` Christopher Spinrath

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57629FBE.7030001@compulab.co.il \
    --to=grinberg@compulab.co.il \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.