Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1320223 > unrolled thread

Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION  quirk Shawn Lin <shawn.lin@rock-chips.com> - 2016-01-28 03:20 +0100
    Re: [RFC PATCH 0/21] Totally remove  SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-28 12:40 +0100
      Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk Ulf Hansson <ulf.hansson@linaro.org> - 2016-01-28 16:10 +0100
        Re: [RFC PATCH 0/21] Totally remove  SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-28 17:00 +0100

#1320223 — Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk

On 2016/1/27 23:07, Ulf Hansson wrote:
> On 27 January 2016 at 14:23, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Wed, Jan 27, 2016 at 02:59:14PM +0200, Adrian Hunter wrote:
>>> In my view Ulf needs to explain how the SDHCI library is going to work,
>>> particularly in the absence of new callbacks.
>>
>> We need to add new callbacks as part of the conversion to a library,
>> otherwise we're very much into a total rewrite from scratch (which
>> I think is far too much work, and prone to errors) or a big flag day
>> to switch everything over (which will require a moritorium on sdhci
>> patches while the effort to switch everything is ongoing.)
>>

Totally agreed. Maybe that is the reason that frighten some volunteers
to make the conversion.

>> Both of those approaches suffer from one huge drawback: there is no
>> way to bisect between them to locate the cause of a regression.
>>

[...]

>>
>> I think what needs to happen here is that Ulf needs to leave such
>> decisions about what is acceptable or unacceptable to those who are
>> trying to convert sdhci to a library, otherwise the conversion will
>> probably never happen... unless Ulf wants to get directly involved
>> in the conversion effort, producing patches to make it happen.
>>
>
> I don't intend to contribute much with actual patches. I am willing to
> help review and also help with expertise around the PM related parts.
>
> I do realize that some callbacks may still be needed, even in the end
> when sdhci has become a pure library. Although, those should be far
> less then those we have today.
>
> Currently I am more or less unable to properly maintain sdhci because
> of it's bad code structure. Therefore I have taken a quite simple
> approach by rejecting new callbacks and quirks, in a way to prevent it
> from being worse.

Ulf, I do understand your situation. sdhci makes you exhausted and no
one seems able to maintain it properly. But preventing it from being
worse doesn't mean making it better, right?

I'am not a experienced sdhci expert, so when I read the sdhci code
for the first time last summer, it does shock me a lot. So many
historical burden it takes with lots kinds of *quirks*, I even cannot
undertand why some quirks are need since git-blame just tell me some
useless info because somebody do some coding-style fix or moving code
here and there, which makes me hard to trace the changes. Another fact,
how to test these changes for diff hardwares?? Without the help of
variant drivers folks, it cannot go a step. If we split our "library
direction" movement, do you think all the variant drivers are willing
to test all the patchsets for it? I don't think so.

Here, I come up with a bold and tentative proposal:
If someone is willing to be the maintainer of sdhci, he/she can create
a separate files and rewrite it to be a library. Then we reject to
accept any new variant drivers to use the old sdhci struct and encourage
it to fit the library one. Then ask the variant drivers who use the old
struct to migrate its code to fit the library. Until all done, remove
the old sdhci. Does that make sense?


To me, the best way forward would be if some of you
> experienced sdhci developers stepped in as a maintainer for it. In
> that way, I can trust the development moving in the "library
> direction" so I can pull back from nacking potential interim sdhci
> callbacks/quirks.
>
> Does it make sense?
>
> Kind regards
> Uffe
>
>
>


-- 
Best Regards
Shawn Lin

[toc] | [next] | [standalone]

#1320596 — Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk

On Thu, 28 Jan 2016 10:17:11 +0800
Shawn Lin <shawn.lin@rock-chips.com> wrote:

> On 2016/1/27 23:07, Ulf Hansson wrote:
> > On 27 January 2016 at 14:23, Russell King - ARM Linux
> > <linux@arm.linux.org.uk> wrote:  
> >> On Wed, Jan 27, 2016 at 02:59:14PM +0200, Adrian Hunter wrote:  
> >>> In my view Ulf needs to explain how the SDHCI library is going to work,
> >>> particularly in the absence of new callbacks.  
> >>
> >> We need to add new callbacks as part of the conversion to a library,
> >> otherwise we're very much into a total rewrite from scratch (which
> >> I think is far too much work, and prone to errors) or a big flag day
> >> to switch everything over (which will require a moritorium on sdhci
> >> patches while the effort to switch everything is ongoing.)

Agreed that won't work. It never works. You simply end up with a bigger
mess, a giant backlog and lots of people sending patches "on appeal" to
Linus/GregKH because the maintainer is being unreasonable. At that point
it all gets messy. It has been done (IDE) where the old core code simply
couldn't cope with the new hardware, but it was a nightmare

> > of it's bad code structure. Therefore I have taken a quite simple
> > approach by rejecting new callbacks and quirks, in a way to prevent it
> > from being worse.  

Which merely guarantees that the problem gets worse, because everyone
just puts their SD patches into Android trees instead and then when that
device is needed in Linux proper the crap hits the fan or people write
uglier and more hideous hacks buried elsewhere.

Eventually something gives way, and it will always be the maintainer,
because everyone needs to get their devices supported. You can guide new
callbacks in constructive ways but not stop them.

> If someone is willing to be the maintainer of sdhci, he/she can create
> a separate files and rewrite it to be a library. Then we reject to
> accept any new variant drivers to use the old sdhci struct and encourage
> it to fit the library one. Then ask the variant drivers who use the old
> struct to migrate its code to fit the library. Until all done, remove
> the old sdhci. Does that make sense?

I don't think it's the best idea. From past experience with other layers
of code the most effective approach (except for IDE which *was* a dead
loss and rewritten) has been something like this

- Take a small function with a quirk in it that one driver uses
- Split it into a clean function and driver specific function, put one in
  the driver and one in the core
- Replace the call to it with a call to ->ops->whatever()
- Add it to the various drivers operations structure as it gets invented

repeat until done.

Some things are shared so you end up moving vendor common quirks to
perhaps an "intel" file or a "designware" file rather than a single
driver, but the theory is the same and it ends up in effect as a class
with some overridden methods.

After this cycle has been repeated a bit you actually have the real
structure of the core code and the areas it diverges are visible. All your
ops-> methods are the points of diverge and clearly labelled. If you want
you can then go through finding all those that fit the pattern where they
diverge in the form

	do_stuff
	standard_method()
	do_stuff

and if the ops callback is not deeply buried look at kicking the whole
method back out completely so it becomes a true library method that
drivers can wrap or call.

It can be done. The 8250 serial driver has been going through this
process for some time (although it didn't start from quite so bad a
position as the sd layer), and the tty layer went through chunks of it
several times in its history.

And above all - it bisects.

Alan

[toc] | [prev] | [next] | [standalone]

#1320773 — Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk

[...]

>
>> > of it's bad code structure. Therefore I have taken a quite simple
>> > approach by rejecting new callbacks and quirks, in a way to prevent it
>> > from being worse.
>
> Which merely guarantees that the problem gets worse, because everyone
> just puts their SD patches into Android trees instead and then when that
> device is needed in Linux proper the crap hits the fan or people write
> uglier and more hideous hacks buried elsewhere.
>
> Eventually something gives way, and it will always be the maintainer,
> because everyone needs to get their devices supported. You can guide new
> callbacks in constructive ways but not stop them.

Well, I did stop them at least temporary.

Although, I have been telling people *why* and also trying to give
some guidelines of how I wanted this to move forward.

I understand some become frustrated from getting patches nacked like this.

In principle I have requested them to help evolving sdhci in a new and
better direction, instead of adding yet more hacks. That of course
requires a deeper understanding of both the mmc core, but also sdhci
in general.

[...]

Also, thanks for sharing your experience in this field. You made some
good points!

Kind regards
Uffe

[toc] | [prev] | [next] | [standalone]

#1320814 — Re: [RFC PATCH 0/21] Totally remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk

On Thu, 28 Jan 2016 16:03:34 +0100
Ulf Hansson <ulf.hansson@linaro.org> wrote:

> [...]
> 
> >  
> >> > of it's bad code structure. Therefore I have taken a quite simple
> >> > approach by rejecting new callbacks and quirks, in a way to prevent it
> >> > from being worse.  
> >
> > Which merely guarantees that the problem gets worse, because everyone
> > just puts their SD patches into Android trees instead and then when that
> > device is needed in Linux proper the crap hits the fan or people write
> > uglier and more hideous hacks buried elsewhere.
> >
> > Eventually something gives way, and it will always be the maintainer,
> > because everyone needs to get their devices supported. You can guide new
> > callbacks in constructive ways but not stop them.  
> 
> Well, I did stop them at least temporary.

I always describe it as "putting a cork in the sewerage pipe". It might
stop it for a bit but

a) you don't want to be too close when it breaks
b) it's not good what happens further up the pipe

> 
> Although, I have been telling people *why* and also trying to give
> some guidelines of how I wanted this to move forward.
> 
> I understand some become frustrated from getting patches nacked like this.
> 
> In principle I have requested them to help evolving sdhci in a new and
> better direction, instead of adding yet more hacks. That of course
> requires a deeper understanding of both the mmc core, but also sdhci
> in general.
> 
> [...]
> 
> Also, thanks for sharing your experience in this field. You made some
> good points!

I'm happy to help try and sort the code out. Not maintain it - my
knowledge of the intricacies of SDHCI is not good enough.

Alan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web