* Fwd: can we remove the quirk SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN ? [not found] <4D68A6A7-2D95-463F-875B-DB52ECCC53F1@marvell.com> @ 2010-11-09 11:45 ` Philip Rakity 2010-11-09 11:55 ` Wolfram Sang 0 siblings, 1 reply; 6+ messages in thread From: Philip Rakity @ 2010-11-09 11:45 UTC (permalink / raw) To: linux-mmc@vger.kernel.org; +Cc: Zhangfei Gao reposting since we are discussing removing quirks. We could partition the quirks into one set for controller quirks and one set for platform quirks. That would give us another 32 quirks. Philip Begin forwarded message: > From: Philip Rakity <prakity@marvell.com> > Date: September 24, 2010 2:31:25 AM PDT > To: Anton Vorontsov <cbouatmailru@gmail.com> > Subject: Re: can we remove the quirk SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN ? > > > you are right -- it would work -- was trying to get rid of quirk since really not needed. Do not agree with Pierre. Understand when registers in sdhci.c need changes quirks are good. getting valves from platform code seems the wrong usage model. > > On Sep 24, 2010, at 2:27 AM, Anton Vorontsov wrote: > >> On Fri, Sep 24, 2010 at 02:15:57AM -0700, Philip Rakity wrote: >>> >>> For our controller the original code would not work, since we get a clock value but the clock value may not be right. The chip sd bus frequency can be adjusted in the platform init code to a higher value. This value needs to be communicated to the sd layer. >>> >>> Are you happy that the patch does not break your code? If so I can submit patch with extended rationale. >> >> Hm. Why would the driver won't work if you do >> specify SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN? Or there's some additional >> patch that you're talking about? >> >> Thanks, >> >>> >>> Philip >>> >>> Eg >>> ________________________________________ >>> From: Anton Vorontsov [cbouatmailru@gmail.com] >>> Sent: Thursday, September 23, 2010 11:59 PM >>> To: Philip Rakity >>> Subject: Re: can we remove the quirk SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN ? >>> >>> On Thu, Sep 23, 2010 at 04:57:41PM -0700, Philip Rakity wrote: >>>> >>>> would be much better if I enclosed the code that compiles. >>>> >>>> Philip >>>> >>>> ==== >>>> >>>> I was looking at my code and then realized the quirk is not needed since host->ops->max_clock is REALLY the test. >>>> >>>> Do you concur ? if so -- want to co sponsor the patch >>>> >>>> I have NOT tried it since do not have your system. >>> >>> See why there is a quirk: >>> >>> http://kerneltrap.org/mailarchive/linux-kernel/2010/2/19/4540115 >>> >>> Thanks, >>> >>>> Philip >>>> >>>> ==== >>>> >>>> b/drivers/mmc/host/sdhci-cns3xxx.c >>>> index b7050b3..27c896d 100644 >>>> --- a/drivers/mmc/host/sdhci-cns3xxx.c >>>> +++ b/drivers/mmc/host/sdhci-cns3xxx.c >>>> @@ -91,7 +91,6 @@ struct sdhci_pltfm_data sdhci_cns3xxx_pdata = { >>>> .quirks = SDHCI_QUIRK_BROKEN_DMA | >>>> SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | >>>> SDHCI_QUIRK_INVERTED_WRITE_PROTECT | >>>> - SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN | >>>> SDHCI_QUIRK_BROKEN_TIMEOUT_VAL | >>>> SDHCI_QUIRK_NONSTANDARD_CLOCK, >>>> }; >>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >>>> index 401527d..3d809f5 100644 >>>> --- a/drivers/mmc/host/sdhci.c >>>> +++ b/drivers/mmc/host/sdhci.c >>>> @@ -1779,18 +1779,20 @@ int sdhci_add_host(struct sdhci_host *host) >>>> mmc_dev(host->mmc)->dma_mask = &host->dma_mask; >>>> } >>>> >>>> - host->max_clk = >>>> - (caps & SDHCI_CLOCK_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT; >>>> - host->max_clk *= 1000000; >>>> - if (host->max_clk == 0 || host->quirks & >>>> - SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN) { >>>> - if (!host->ops->get_max_clock) { >>>> - printk(KERN_ERR >>>> - "%s: Hardware doesn't specify base clock " >>>> - "frequency.\n", mmc_hostname(mmc)); >>>> - return -ENODEV; >>>> - } >>>> + if (host->ops->get_max_clock) >>>> host->max_clk = host->ops->get_max_clock(host); >>>> + else { >>>> + host->max_clk = >>>> + (caps & SDHCI_CLOCK_BASE_MASK) >>>> + >> SDHCI_CLOCK_BASE_SHIFT; >>>> + host->max_clk *= 1000000; >>>> + } >>>> + >>>> + if (host->max_clk == 0) { >>>> + printk(KERN_ERR >>>> + "%s: Hardware doesn't specify base clock " >>>> + "frequency.\n", mmc_hostname(mmc)); >>>> + return -ENODEV; >>>> } >>>> >>>> host->timeout_clk = >>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h >>>> index d316bc7..d7c85fb 100644 >>>> --- a/drivers/mmc/host/sdhci.h >>>> +++ b/drivers/mmc/host/sdhci.h >>>> @@ -237,8 +237,6 @@ struct sdhci_host { >>>> #define SDHCI_QUIRK_DELAY_AFTER_POWER (1<<23) >>>> /* Controller uses SDCLK instead of TMCLK for data timeouts */ >>>> #define SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK (1<<24) >>>> -/* Controller reports wrong base clock capability */ >>>> -#define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN (1<<25) >>>> /* Controller cannot support End Attribute in NOP ADMA descriptor */ >>>> #define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC (1<<26) >>>> /* Controller is missing device caps. Use caps provided by host */ >>>> >>> >>> -- >>> Anton Vorontsov >>> email: cbouatmailru@gmail.com >>> irc://irc.freenode.net/bd2 >> >> -- >> Anton Vorontsov >> email: cbouatmailru@gmail.com >> irc://irc.freenode.net/bd2 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fwd: can we remove the quirk SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN ? 2010-11-09 11:45 ` Fwd: can we remove the quirk SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN ? Philip Rakity @ 2010-11-09 11:55 ` Wolfram Sang 2010-11-09 12:02 ` Philip Rakity 0 siblings, 1 reply; 6+ messages in thread From: Wolfram Sang @ 2010-11-09 11:55 UTC (permalink / raw) To: Philip Rakity; +Cc: linux-mmc@vger.kernel.org, Zhangfei Gao [-- Attachment #1: Type: text/plain, Size: 668 bytes --] On Tue, Nov 09, 2010 at 03:45:15AM -0800, Philip Rakity wrote: > > reposting since we are discussing removing quirks. > > We could partition the quirks into one set for controller quirks and one set for platform quirks. That would give us another 32 quirks. IMHO we need less quirks, not more. Adding the number of potential quirks just delays the underlying problem. Also, unless someone enlightens me, I don't think there are platform quirks, only platform_data? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: can we remove the quirk SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN ? 2010-11-09 11:55 ` Wolfram Sang @ 2010-11-09 12:02 ` Philip Rakity 2010-11-09 12:08 ` Alan Cox 2010-11-09 12:21 ` Wolfram Sang 0 siblings, 2 replies; 6+ messages in thread From: Philip Rakity @ 2010-11-09 12:02 UTC (permalink / raw) To: Wolfram Sang; +Cc: linux-mmc@vger.kernel.org, Zhangfei Gao Doesn't the platform data hold the specific board specific quirks ? Can you provide an example of how you see the platform data being used. I think we are saying the same thing using different words but a code snippet would clarify this. Philip On Nov 9, 2010, at 3:55 AM, Wolfram Sang wrote: > On Tue, Nov 09, 2010 at 03:45:15AM -0800, Philip Rakity wrote: >> >> reposting since we are discussing removing quirks. >> >> We could partition the quirks into one set for controller quirks and one set for platform quirks. That would give us another 32 quirks. > > IMHO we need less quirks, not more. Adding the number of potential > quirks just delays the underlying problem. Also, unless someone > enlightens me, I don't think there are platform quirks, only > platform_data? > > Regards, > > Wolfram > > -- > Pengutronix e.K. | Wolfram Sang | > Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: can we remove the quirk SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN ? 2010-11-09 12:02 ` Philip Rakity @ 2010-11-09 12:08 ` Alan Cox 2010-11-09 12:27 ` Wolfram Sang 2010-11-09 12:21 ` Wolfram Sang 1 sibling, 1 reply; 6+ messages in thread From: Alan Cox @ 2010-11-09 12:08 UTC (permalink / raw) To: Philip Rakity; +Cc: Wolfram Sang, linux-mmc@vger.kernel.org, Zhangfei Gao I think I agree with Wolfram. Perhaps a much better model would be "Anyone who adds a quirk will have to figure out how to remove one" Several are trivial to remove and generalise into a couple of function overrides. Alan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: can we remove the quirk SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN ? 2010-11-09 12:08 ` Alan Cox @ 2010-11-09 12:27 ` Wolfram Sang 0 siblings, 0 replies; 6+ messages in thread From: Wolfram Sang @ 2010-11-09 12:27 UTC (permalink / raw) To: Alan Cox; +Cc: Philip Rakity, linux-mmc@vger.kernel.org, Zhangfei Gao [-- Attachment #1: Type: text/plain, Size: 709 bytes --] On Tue, Nov 09, 2010 at 12:08:35PM +0000, Alan Cox wrote: > I think I agree with Wolfram. Perhaps a much better model would be > > "Anyone who adds a quirk will have to figure out how to remove one" > > Several are trivial to remove and generalise into a couple of function > overrides. My favourite would be one concentrated action to remove a couple of those quirks while adding well defined hooks to reduce the need of new ones. I have a few ideas and am currently trying a few options to get it funded :) Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: can we remove the quirk SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN ? 2010-11-09 12:02 ` Philip Rakity 2010-11-09 12:08 ` Alan Cox @ 2010-11-09 12:21 ` Wolfram Sang 1 sibling, 0 replies; 6+ messages in thread From: Wolfram Sang @ 2010-11-09 12:21 UTC (permalink / raw) To: Philip Rakity; +Cc: linux-mmc@vger.kernel.org, Zhangfei Gao [-- Attachment #1: Type: text/plain, Size: 544 bytes --] Hi Philip, > Doesn't the platform data hold the specific board specific quirks ? If we agree on not calling it "quirks" but "configuration", then we may in deed be talking about the very same thing :) > Can you provide an example of how you see the platform data being > used. Sad thing is that I don't have a board using an 8 bit width bus :( Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-11-09 12:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <4D68A6A7-2D95-463F-875B-DB52ECCC53F1@marvell.com>
2010-11-09 11:45 ` Fwd: can we remove the quirk SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN ? Philip Rakity
2010-11-09 11:55 ` Wolfram Sang
2010-11-09 12:02 ` Philip Rakity
2010-11-09 12:08 ` Alan Cox
2010-11-09 12:27 ` Wolfram Sang
2010-11-09 12:21 ` Wolfram Sang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox