<div dir="ltr"><span style="font-size:12.8000001907349px">On Monday 27 April 2015 21:25:20 Suman Tripathi wrote:<br>> > On Monday 27 April 2015 20:33:25 Suman Tripathi wrote:<br>> > > > On Tuesday 21 April 2015 21:12:39 Suman Tripathi wrote:<br></span><span style="font-size:12.8000001907349px">> > > > > +                       host->quirks |= SDHCI_QUIRK_BROKEN_DMA;<br>> > > > > +<br>> > > > > +               if (of_get_property(np, "no-cmd23", NULL))<br>> > > > > +                       host->quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23;<br>> > > > ><br>> > > > >                 if (of_get_property(np, "no-1-8-v", NULL))<br>> > > > ><br>> > > > >                         host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;<br>> > > ><br>> > > > Any property you add needs to be documented in the DT binding.<br>> > > > If possible, add generic properties for each bug you have <span>mmc</span>.txt<br>> > > > rather than the driver specific sdhci.txt, and implement the<br>> > ><br>> > > I will add the binding in <span>mmc</span>.txt. I thought this was present but not.<br>> > ><br>> > > > parsing in a common function that is used for all <span>mmc</span> hosts.<br>> > ><br>> > > As per mine understanding the sdhci_get_of_porperty is a common<br>> > > parsing function  . Am I wrong ??<br>><br><br></span><span style="font-size:12.8000001907349px">A small side note: please fix your email client to use proper attribution</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">of the citations. The way you reply, nobody knows what you are saying</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">compare to what you quote. Also, reduce the quotation to the parts you</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">are replying to.</span><div><br></div><div>Ok .  sorry for that ..<br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px"><br>> > No, this is only used for sdhci, not for the other controllers.<br>><br>> But our's is a SHCI variant so I added it in this file.<br><br></span><span style="font-size:12.8000001907349px">That's my point: a lot of the bugs are independent of the specific</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">host controller and could happen with any one of them. We want to</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">ensure that nobody tries to add another property with similar</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">semantics and a different name just because they are using a</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">different driver.</span></div><div><br></div><div>Then I am not finding a reason why we have sdhci_get_of_property function ?? .</div><div> I added a generic names like broken-adma that everyone can reuse it.  I made mistake of not adding it in the binding.</div><div><br></div><div>For eg : broken-cd is not added by me but I can use it. So I added something like broken-adma as it was not present.</div><div><span style="font-size:12.8000001907349px"><br>> > > An alternative would be to set all these bits based on the compatible<br>> > > string of your host, if that is the only one that has all these bugs.<br>> ><br>> > The host driver  (arasan) is reused but this quirks are needed due to<br>> > board issues. so I have a control over dtb only to fix this.<br>><br>> What is the nature of the bug on that board? Is there a different<br>> way to describe that without introducing six new properties?<br>><br>> Sorry it is board and IP as well SoC errata's,<br>><br>> 1. Delay after power is required due to some voltage issues that will<br>> be fixed in next board revision<br><br></span><span style="font-size:12.8000001907349px">This is clearly not sdhci-specific, so make that a generic property</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">for all </span><span style="font-size:12.8000001907349px;background-color:rgb(255,255,255)">mmc</span><span style="font-size:12.8000001907349px">.</span></div><div>okay.<br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px"><br>> 2. We need to support PIO mode as of now because DMA or ADMA requires<br>> some kind of translation driver that I am working on.<br><br></span><span style="font-size:12.8000001907349px">But this does not describe the hardware properties. Don't add properties</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">that describe the lack of a kernel driver. If you can't do DMA yet,</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">use a dma-ranges property that lists one empty range to prevent</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">dma_set_mask() from working, so it will fall back to PIO mode. You</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">may have to fix the driver if that doesn't already work.</span></div><div><br></div><div>The generic sdhc framework doesn't have this capabiltiy. It uses the quirks to identify the broken DMA and ADMA modes even</div><div>if the controller is capable of.<br><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">What kind of driver do you need here?</span></div><div>For DMA and adma we need some 32 bit to 64 bit translation driver.  The existing arasan driver only support 32 bit.<br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px"><br>> 3. The version of arasan variant we have in our SoC doesn't have the<br>> HISPD  bit field in HI-SPEED SD card. So this makes HI-SPEED sdcard<br>> work.<br>><br>> 4. NO_CMD23 is required for eMMC cards.<br>><br>> These are not new properties.  Only the fact is I am using it for our<br>> SoC from dtb. These quirks are already there in <span>mmc</span> common framework.<br>> Nothing is new.<br><br></span><span style="font-size:12.8000001907349px">Are you sure that you have version 8.9a of the Arasan SDHCI? </span></div><div><span style="font-size:12.8000001907349px">Yes</span></div><div><span style="font-size:12.8000001907349px">This sounds</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">like version specific quirks, so they are probably present in each</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">SoC that uses the same version.</span></div><div><span style="font-size:12.8000001907349px">Not sure.<br></span><div style="font-size:12.8000001907349px"><div><img src="https://ssl.gstatic.com/ui/v1/icons/mail/images/cleardot.gif"></div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Apr 28, 2015 at 1:19 AM, Arnd Bergmann <span dir="ltr"><<a href="mailto:arnd@arndb.de" target="_blank">arnd@arndb.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Monday 27 April 2015 21:25:20 Suman Tripathi wrote:<br>
> > On Monday 27 April 2015 20:33:25 Suman Tripathi wrote:<br>
> > > > On Tuesday 21 April 2015 21:12:39 Suman Tripathi wrote:<br>
</span><span class="">> > > > > +                       host->quirks |= SDHCI_QUIRK_BROKEN_DMA;<br>
> > > > > +<br>
> > > > > +               if (of_get_property(np, "no-cmd23", NULL))<br>
> > > > > +                       host->quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23;<br>
> > > > ><br>
> > > > >                 if (of_get_property(np, "no-1-8-v", NULL))<br>
> > > > ><br>
> > > > >                         host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;<br>
> > > ><br>
> > > > Any property you add needs to be documented in the DT binding.<br>
> > > > If possible, add generic properties for each bug you have mmc.txt<br>
> > > > rather than the driver specific sdhci.txt, and implement the<br>
> > ><br>
> > > I will add the binding in mmc.txt. I thought this was present but not.<br>
> > ><br>
> > > > parsing in a common function that is used for all mmc hosts.<br>
> > ><br>
> > > As per mine understanding the sdhci_get_of_porperty is a common<br>
> > > parsing function  . Am I wrong ??<br>
><br>
<br>
</span>A small side note: please fix your email client to use proper attribution<br>
of the citations. The way you reply, nobody knows what you are saying<br>
compare to what you quote. Also, reduce the quotation to the parts you<br>
are replying to.<br>
<span class=""><br>
> > No, this is only used for sdhci, not for the other controllers.<br>
><br>
> But our's is a SHCI variant so I added it in this file.<br>
<br>
</span>That's my point: a lot of the bugs are independent of the specific<br>
host controller and could happen with any one of them. We want to<br>
ensure that nobody tries to add another property with similar<br>
semantics and a different name just because they are using a<br>
different driver.<br>
<span class=""><br>
> > > An alternative would be to set all these bits based on the compatible<br>
> > > string of your host, if that is the only one that has all these bugs.<br>
> ><br>
> > The host driver  (arasan) is reused but this quirks are needed due to<br>
> > board issues. so I have a control over dtb only to fix this.<br>
><br>
> What is the nature of the bug on that board? Is there a different<br>
> way to describe that without introducing six new properties?<br>
><br>
> Sorry it is board and IP as well SoC errata's,<br>
><br>
> 1. Delay after power is required due to some voltage issues that will<br>
> be fixed in next board revision<br>
<br>
</span>This is clearly not sdhci-specific, so make that a generic property<br>
for all mmc.<br>
<span class=""><br>
> 2. We need to support PIO mode as of now because DMA or ADMA requires<br>
> some kind of translation driver that I am working on.<br>
<br>
</span>But this does not describe the hardware properties. Don't add properties<br>
that describe the lack of a kernel driver. If you can't do DMA yet,<br>
use a dma-ranges property that lists one empty range to prevent<br>
dma_set_mask() from working, so it will fall back to PIO mode. You<br>
may have to fix the driver if that doesn't already work.<br>
<br>
What kind of driver do you need here?<br>
<span class=""><br>
> 3. The version of arasan variant we have in our SoC doesn't have the<br>
> HISPD  bit field in HI-SPEED SD card. So this makes HI-SPEED sdcard<br>
> work.<br>
><br>
> 4. NO_CMD23 is required for eMMC cards.<br>
><br>
> These are not new properties.  Only the fact is I am using it for our<br>
> SoC from dtb. These quirks are already there in mmc common framework.<br>
> Nothing is new.<br>
<br>
</span>Are you sure that you have version 8.9a of the Arasan SDHCI? This sounds<br>
like version specific quirks, so they are probably present in each<br>
SoC that uses the same version.<br>
<span class="HOEnZb"><font color="#888888"><br>
        Arnd<br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr"><div>Thanks,</div><div>with regards,</div>Suman Tripathi</div></div>
</div>