Message ID | 1450067067-44869-6-git-send-email-yangbo.lu@freescale.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
On 14 December 2015 at 05:24, Yangbo Lu <yangbo.lu@freescale.com> wrote: > The sdhci-of-esdhc driver needs the GUTS driver support > to access the global utilities registers for SVR value. > So we select FSL_GUTS for MMC_SDHCI_OF_ESDHC here. > > Signed-off-by: Yangbo Lu <yangbo.lu@freescale.com> > --- > Changes for v2: > - None > Changes for v3: > - None > Changes for v4: > - Added this patch > --- > drivers/mmc/host/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > index 1526b8a..df57c14 100644 > --- a/drivers/mmc/host/Kconfig > +++ b/drivers/mmc/host/Kconfig > @@ -142,6 +142,8 @@ config MMC_SDHCI_OF_ESDHC > depends on MMC_SDHCI_PLTFM > depends on PPC || ARCH_MXC || ARCH_LAYERSCAPE > select MMC_SDHCI_IO_ACCESSORS > + select SOC_FSL > + select FSL_GUTS This is weird. First, perhaps it would make sense to have stub functions for those the FSL_GUTS driver provides via its API, thus the above wouldn't be required at all. Of course this makes only sense if you think there are/could be configurations for a cross SOC driver which don't need the GUTS driver. Second, even if you think the stubs above is a bad idea, I would from the top-level Kconfig for your platform, add the needed "selects" as I think it's where it belongs and then change this to "depends on" instead. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2015-12-14 at 14:08 +0100, Ulf Hansson wrote: > On 14 December 2015 at 05:24, Yangbo Lu <yangbo.lu@freescale.com> wrote: > > The sdhci-of-esdhc driver needs the GUTS driver support > > to access the global utilities registers for SVR value. > > So we select FSL_GUTS for MMC_SDHCI_OF_ESDHC here. > > > > Signed-off-by: Yangbo Lu <yangbo.lu@freescale.com> > > --- > > Changes for v2: > > - None > > Changes for v3: > > - None > > Changes for v4: > > - Added this patch > > --- > > drivers/mmc/host/Kconfig | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > > index 1526b8a..df57c14 100644 > > --- a/drivers/mmc/host/Kconfig > > +++ b/drivers/mmc/host/Kconfig > > @@ -142,6 +142,8 @@ config MMC_SDHCI_OF_ESDHC > > depends on MMC_SDHCI_PLTFM > > depends on PPC || ARCH_MXC || ARCH_LAYERSCAPE > > select MMC_SDHCI_IO_ACCESSORS > > + select SOC_FSL > > + select FSL_GUTS > > This is weird. > > First, perhaps it would make sense to have stub functions for those > the FSL_GUTS driver provides via its API, thus the above wouldn't be > required at all. Of course this makes only sense if you think there > are/could be configurations for a cross SOC driver which don't need > the GUTS driver. > > Second, even if you think the stubs above is a bad idea, I would from > the top-level Kconfig for your platform, add the needed "selects" as I > think it's where it belongs and then change this to "depends on" > instead. Why is it weird for a driver to select another driver that it makes calls to? Much easier to do it here than in all the platforms that use this driver. And I think stubs for reading SVR is quite a bad idea. It'll make the driver build but it will silently not be able to apply SVR-based workarounds. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2015-12-14 at 12:24 +0800, Yangbo Lu wrote: > The sdhci-of-esdhc driver needs the GUTS driver support > to access the global utilities registers for SVR value. > So we select FSL_GUTS for MMC_SDHCI_OF_ESDHC here. > > Signed-off-by: Yangbo Lu <yangbo.lu@freescale.com> > --- > Changes for v2: > - None > Changes for v3: > - None > Changes for v4: > - Added this patch > --- > drivers/mmc/host/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > index 1526b8a..df57c14 100644 > --- a/drivers/mmc/host/Kconfig > +++ b/drivers/mmc/host/Kconfig > @@ -142,6 +142,8 @@ config MMC_SDHCI_OF_ESDHC > depends on MMC_SDHCI_PLTFM > depends on PPC || ARCH_MXC || ARCH_LAYERSCAPE > select MMC_SDHCI_IO_ACCESSORS > + select SOC_FSL > + select FSL_GUTS > help > This selects the Freescale eSDHC controller support. > What is "SOC_FSL"? If you mean "FSL_SOC", why are you selecting it? -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[...] >> > --- a/drivers/mmc/host/Kconfig >> > +++ b/drivers/mmc/host/Kconfig >> > @@ -142,6 +142,8 @@ config MMC_SDHCI_OF_ESDHC >> > depends on MMC_SDHCI_PLTFM >> > depends on PPC || ARCH_MXC || ARCH_LAYERSCAPE >> > select MMC_SDHCI_IO_ACCESSORS >> > + select SOC_FSL >> > + select FSL_GUTS >> >> This is weird. >> >> First, perhaps it would make sense to have stub functions for those >> the FSL_GUTS driver provides via its API, thus the above wouldn't be >> required at all. Of course this makes only sense if you think there >> are/could be configurations for a cross SOC driver which don't need >> the GUTS driver. >> >> Second, even if you think the stubs above is a bad idea, I would from >> the top-level Kconfig for your platform, add the needed "selects" as I >> think it's where it belongs and then change this to "depends on" >> instead. > > Why is it weird for a driver to select another driver that it makes calls to? > Much easier to do it here than in all the platforms that use this driver. Because using "select" will not consider the dependencies for the new selected Kconfig option. I can imagine that it might become a problem, sooner or later. So, "select" shall be used by care and in this case I think we can cope fine with using "depends on". > > And I think stubs for reading SVR is quite a bad idea. It'll make the driver > build but it will silently not be able to apply SVR-based workarounds. It doesn't have to be "silent", the driver can return an error (and print error messages) from its ->probe() method, if the calls to the GUTS driver fails. Anyway, I mentioned this idea only to understand the need for *optional* GUTS supports. Perhaps there is a cross SOC drivers that for some platforms depends on GUTS but on others it doesn't. Maybe that isn't case then!? Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2015-12-15 at 10:46 +0100, Ulf Hansson wrote: > [...] > > > > --- a/drivers/mmc/host/Kconfig > > > > +++ b/drivers/mmc/host/Kconfig > > > > @@ -142,6 +142,8 @@ config MMC_SDHCI_OF_ESDHC > > > > depends on MMC_SDHCI_PLTFM > > > > depends on PPC || ARCH_MXC || ARCH_LAYERSCAPE > > > > select MMC_SDHCI_IO_ACCESSORS > > > > + select SOC_FSL > > > > + select FSL_GUTS > > > > > > This is weird. > > > > > > First, perhaps it would make sense to have stub functions for those > > > the FSL_GUTS driver provides via its API, thus the above wouldn't be > > > required at all. Of course this makes only sense if you think there > > > are/could be configurations for a cross SOC driver which don't need > > > the GUTS driver. > > > > > > Second, even if you think the stubs above is a bad idea, I would from > > > the top-level Kconfig for your platform, add the needed "selects" as I > > > think it's where it belongs and then change this to "depends on" > > > instead. > > > > Why is it weird for a driver to select another driver that it makes calls > > to? > > Much easier to do it here than in all the platforms that use this driver. > > Because using "select" will not consider the dependencies for the new > selected Kconfig option. I can imagine that it might become a problem, > sooner or later. It's not a problem as long as the selected option's dependencies (if any) are selected, or depended on by the selecting driver. I wouldn't expect the FSL_GUTS driver to depend on anything other than basic OF support. > So, "select" shall be used by care and in this case I think we can > cope fine with using "depends on". What does select exist for if not situations like this? What "care" is missing? -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16 December 2015 at 23:48, Scott Wood <scottwood@freescale.com> wrote: > On Tue, 2015-12-15 at 10:46 +0100, Ulf Hansson wrote: >> [...] >> > > > --- a/drivers/mmc/host/Kconfig >> > > > +++ b/drivers/mmc/host/Kconfig >> > > > @@ -142,6 +142,8 @@ config MMC_SDHCI_OF_ESDHC >> > > > depends on MMC_SDHCI_PLTFM >> > > > depends on PPC || ARCH_MXC || ARCH_LAYERSCAPE >> > > > select MMC_SDHCI_IO_ACCESSORS >> > > > + select SOC_FSL >> > > > + select FSL_GUTS >> > > >> > > This is weird. >> > > >> > > First, perhaps it would make sense to have stub functions for those >> > > the FSL_GUTS driver provides via its API, thus the above wouldn't be >> > > required at all. Of course this makes only sense if you think there >> > > are/could be configurations for a cross SOC driver which don't need >> > > the GUTS driver. >> > > >> > > Second, even if you think the stubs above is a bad idea, I would from >> > > the top-level Kconfig for your platform, add the needed "selects" as I >> > > think it's where it belongs and then change this to "depends on" >> > > instead. >> > >> > Why is it weird for a driver to select another driver that it makes calls >> > to? >> > Much easier to do it here than in all the platforms that use this driver. >> >> Because using "select" will not consider the dependencies for the new >> selected Kconfig option. I can imagine that it might become a problem, >> sooner or later. > > It's not a problem as long as the selected option's dependencies (if any) are > selected, or depended on by the selecting driver. I wouldn't expect the > FSL_GUTS driver to depend on anything other than basic OF support. > >> So, "select" shall be used by care and in this case I think we can >> cope fine with using "depends on". > > What does select exist for if not situations like this? What "care" is > missing? > The GUTS driver is SoC specific driver. If/when dependencies are added for that driver it will break the usage of the select for the MMC host driver. No, thanks it's fragile! To me, the best way to use select is either from top-level platform Kconfig or where it's better isolated, perhaps within a subsystem. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[...] >> >> And I think stubs for reading SVR is quite a bad idea. It'll make the driver >> build but it will silently not be able to apply SVR-based workarounds. > > It doesn't have to be "silent", the driver can return an error (and > print error messages) from its ->probe() method, if the calls to the > GUTS driver fails. > > Anyway, I mentioned this idea only to understand the need for > *optional* GUTS supports. Perhaps there is a cross SOC drivers that > for some platforms depends on GUTS but on others it doesn't. > > Maybe that isn't case then!? Can you please answer this question!? According to the earlier versions of this patchset and from your comments [1], it *do* seems like the GUTS driver may be optional and thus stubs could address this. Kind regards Uffe [1] http://www.spinics.net/lists/linux-mmc/msg34412.html -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBVbGYgSGFuc3NvbiBbbWFpbHRv OnVsZi5oYW5zc29uQGxpbmFyby5vcmddDQo+IFNlbnQ6IFRodXJzZGF5LCBEZWNlbWJlciAxNywg MjAxNSA3OjMxIFBNDQo+IFRvOiBTY290dCBXb29kDQo+IENjOiBMdSBZYW5nYm8tQjQ3MDkzOyBs aW51eC1tbWM7IFhpZSBYaWFvYm8tUjYzMDYxOyBMZW8gbGkNCj4gU3ViamVjdDogUmU6IFt2NCwg NS82XSBtbWM6IGtjb25maWc6IHNlbGVjdCBGU0xfR1VUUyBmb3INCj4gTU1DX1NESENJX09GX0VT REhDDQo+IA0KPiBbLi4uXQ0KPiANCj4gPj4NCj4gPj4gQW5kIEkgdGhpbmsgc3R1YnMgZm9yIHJl YWRpbmcgU1ZSIGlzIHF1aXRlIGEgYmFkIGlkZWEuICBJdCdsbCBtYWtlDQo+ID4+IHRoZSBkcml2 ZXIgYnVpbGQgYnV0IGl0IHdpbGwgc2lsZW50bHkgbm90IGJlIGFibGUgdG8gYXBwbHkgU1ZSLWJh c2VkDQo+IHdvcmthcm91bmRzLg0KPiA+DQo+ID4gSXQgZG9lc24ndCBoYXZlIHRvIGJlICJzaWxl bnQiLCB0aGUgZHJpdmVyIGNhbiByZXR1cm4gYW4gZXJyb3IgKGFuZA0KPiA+IHByaW50IGVycm9y IG1lc3NhZ2VzKSBmcm9tIGl0cyAtPnByb2JlKCkgbWV0aG9kLCBpZiB0aGUgY2FsbHMgdG8gdGhl DQo+ID4gR1VUUyBkcml2ZXIgZmFpbHMuDQo+ID4NCj4gPiBBbnl3YXksIEkgbWVudGlvbmVkIHRo aXMgaWRlYSBvbmx5IHRvIHVuZGVyc3RhbmQgdGhlIG5lZWQgZm9yDQo+ID4gKm9wdGlvbmFsKiBH VVRTIHN1cHBvcnRzLiBQZXJoYXBzIHRoZXJlIGlzIGEgY3Jvc3MgU09DIGRyaXZlcnMgdGhhdA0K PiA+IGZvciBzb21lIHBsYXRmb3JtcyBkZXBlbmRzIG9uIEdVVFMgYnV0IG9uIG90aGVycyBpdCBk b2Vzbid0Lg0KPiA+DQo+ID4gTWF5YmUgdGhhdCBpc24ndCBjYXNlIHRoZW4hPw0KPiANCj4gQ2Fu IHlvdSBwbGVhc2UgYW5zd2VyIHRoaXMgcXVlc3Rpb24hPw0KPiANCj4gQWNjb3JkaW5nIHRvIHRo ZSBlYXJsaWVyIHZlcnNpb25zIG9mIHRoaXMgcGF0Y2hzZXQgYW5kIGZyb20geW91ciBjb21tZW50 cw0KPiBbMV0sIGl0ICpkbyogc2VlbXMgbGlrZSB0aGUgR1VUUyBkcml2ZXIgbWF5IGJlIG9wdGlv bmFsIGFuZCB0aHVzIHN0dWJzDQo+IGNvdWxkIGFkZHJlc3MgdGhpcy4NCj4gDQo+IEtpbmQgcmVn YXJkcw0KPiBVZmZlDQo+IA0KPiBbMV0NCj4gaHR0cDovL3d3dy5zcGluaWNzLm5ldC9saXN0cy9s aW51eC1tbWMvbXNnMzQ0MTIuaHRtbA0KDQpbTHUgWWFuZ2JvLUI0NzA5M10gSGkgU2NvdHQgYW5k IFVmZmUsIA0KSW4gdGhlIGVhcmxpZXIgdmVyc2lvbiwgSSdkIGxpa2UgdG8gdXNlIHN5c2NvbiBz dXBwb3J0IGFuZCBvbmx5IGFkZCAnc3lzY29uJyBjb21wYXRpYmxlIGluIHRoZSBkdHMgd2hvc2Ug ZVNESEMgbmVlZHMgdG8gdXNlIGl0IHRvIGdldCBTVlIuDQpCdXQgSSBuZXZlciB0aG91Z2h0IHRo aXMgaGFkIGNhdXNlZCBzbyBtdWNoIGRpc2N1c3Npb24uLi4gOigNCkNvdWxkIHdlIHJlYWNoIGFu IGFncmVlbWVudCBhYm91dCB0aGUgJ29wdGlvbmFsJyBvciBub3QgJ29wdGlvbmFsJz8NCg0KVGhh bmsgeW91IHNvIG11Y2ghDQoNCiANCg0KDQoNCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 28 December 2015 at 11:26, Yangbo Lu <yangbo.lu@nxp.com> wrote: >> -----Original Message----- >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org] >> Sent: Thursday, December 17, 2015 7:31 PM >> To: Scott Wood >> Cc: Lu Yangbo-B47093; linux-mmc; Xie Xiaobo-R63061; Leo li >> Subject: Re: [v4, 5/6] mmc: kconfig: select FSL_GUTS for >> MMC_SDHCI_OF_ESDHC >> >> [...] >> >> >> >> >> And I think stubs for reading SVR is quite a bad idea. It'll make >> >> the driver build but it will silently not be able to apply SVR-based >> workarounds. >> > >> > It doesn't have to be "silent", the driver can return an error (and >> > print error messages) from its ->probe() method, if the calls to the >> > GUTS driver fails. >> > >> > Anyway, I mentioned this idea only to understand the need for >> > *optional* GUTS supports. Perhaps there is a cross SOC drivers that >> > for some platforms depends on GUTS but on others it doesn't. >> > >> > Maybe that isn't case then!? >> >> Can you please answer this question!? >> >> According to the earlier versions of this patchset and from your comments >> [1], it *do* seems like the GUTS driver may be optional and thus stubs >> could address this. >> >> Kind regards >> Uffe >> >> [1] >> http://www.spinics.net/lists/linux-mmc/msg34412.html > > [Lu Yangbo-B47093] Hi Scott and Uffe, > In the earlier version, I'd like to use syscon support and only add 'syscon' compatible in the dts whose eSDHC needs to use it to get SVR. > But I never thought this had caused so much discussion... :( Sorry, I understand your frustration but that's life sometimes. :-) To me, the syscon solution is more elegant... > Could we reach an agreement about the 'optional' or not 'optional'? ...but I am fine with the current approach as well, as long as my recent comments gets addressed. Let's make it optional. Address Scott's and my review-comments, get other peoples ack for the non-mmc parts, then I will happily pick up the patches. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2015-12-17 at 12:30 +0100, Ulf Hansson wrote: > [...] > > > > > > > And I think stubs for reading SVR is quite a bad idea. It'll make the > > > driver > > > build but it will silently not be able to apply SVR-based workarounds. > > > > It doesn't have to be "silent", the driver can return an error (and > > print error messages) from its ->probe() method, if the calls to the > > GUTS driver fails. > > > > Anyway, I mentioned this idea only to understand the need for > > *optional* GUTS supports. Perhaps there is a cross SOC drivers that > > for some platforms depends on GUTS but on others it doesn't. > > > > Maybe that isn't case then!? > > Can you please answer this question!? > > According to the earlier versions of this patchset and from your > comments [1], it *do* seems like the GUTS driver may be optional and > thus stubs could address this. I'd rather it not be optional at build time for the reason I explained above. It would be too easy for users to accidentally not enable the guts driver and miss the workaround. Even if an error is printed it's easy to miss among all the boot spam -- and if there's any legitimate reason to not have the driver enabled then why would that be an "error"? If the guts driver fails to bind (e.g. running in a VM, or a platform the guts driver doesn't recognize) that's another matter, but that should be handled by an error check in the guts driver, not a build-time stub. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2015-12-17 at 12:25 +0100, Ulf Hansson wrote: > On 16 December 2015 at 23:48, Scott Wood <scottwood@freescale.com> wrote: > > On Tue, 2015-12-15 at 10:46 +0100, Ulf Hansson wrote: > > > [...] > > > > > > --- a/drivers/mmc/host/Kconfig > > > > > > +++ b/drivers/mmc/host/Kconfig > > > > > > @@ -142,6 +142,8 @@ config MMC_SDHCI_OF_ESDHC > > > > > > depends on MMC_SDHCI_PLTFM > > > > > > depends on PPC || ARCH_MXC || ARCH_LAYERSCAPE > > > > > > select MMC_SDHCI_IO_ACCESSORS > > > > > > + select SOC_FSL > > > > > > + select FSL_GUTS > > > > > > > > > > This is weird. > > > > > > > > > > First, perhaps it would make sense to have stub functions for those > > > > > the FSL_GUTS driver provides via its API, thus the above wouldn't be > > > > > required at all. Of course this makes only sense if you think there > > > > > are/could be configurations for a cross SOC driver which don't need > > > > > the GUTS driver. > > > > > > > > > > Second, even if you think the stubs above is a bad idea, I would > > > > > from > > > > > the top-level Kconfig for your platform, add the needed "selects" as > > > > > I > > > > > think it's where it belongs and then change this to "depends on" > > > > > instead. > > > > > > > > Why is it weird for a driver to select another driver that it makes > > > > calls > > > > to? > > > > Much easier to do it here than in all the platforms that use this > > > > driver. > > > > > > Because using "select" will not consider the dependencies for the new > > > selected Kconfig option. I can imagine that it might become a problem, > > > sooner or later. > > > > It's not a problem as long as the selected option's dependencies (if any) > > are > > selected, or depended on by the selecting driver. I wouldn't expect the > > FSL_GUTS driver to depend on anything other than basic OF support. > > > > > So, "select" shall be used by care and in this case I think we can > > > cope fine with using "depends on". > > > > What does select exist for if not situations like this? What "care" is > > missing? > > > > The GUTS driver is SoC specific driver. If/when dependencies are added > for that driver it will break the usage of the select for the MMC host > driver. No, thanks it's fragile! > > To me, the best way to use select is either from top-level platform > Kconfig or where it's better isolated, perhaps within a subsystem. The guts driver is not going to depend on anything that the esdhc driver shouldn't already be depending on (why does MMC_SDHCI_OF_ESDHC not currently depend on OF?) and if the guts driver ever did grow a dependency that isn't met but it is selected anyway, kconfig will warn. I realize that growing a guts dependency list in every user would be bad but I just don't see such dependencies happening. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2015-12-28 at 13:10 +0100, Ulf Hansson wrote: > On 28 December 2015 at 11:26, Yangbo Lu <yangbo.lu@nxp.com> wrote: > > > -----Original Message----- > > > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > > > Sent: Thursday, December 17, 2015 7:31 PM > > > To: Scott Wood > > > Cc: Lu Yangbo-B47093; linux-mmc; Xie Xiaobo-R63061; Leo li > > > Subject: Re: [v4, 5/6] mmc: kconfig: select FSL_GUTS for > > > MMC_SDHCI_OF_ESDHC > > > > > > [...] > > > > > > > > > > > > > And I think stubs for reading SVR is quite a bad idea. It'll make > > > > > the driver build but it will silently not be able to apply SVR-based > > > workarounds. > > > > > > > > It doesn't have to be "silent", the driver can return an error (and > > > > print error messages) from its ->probe() method, if the calls to the > > > > GUTS driver fails. > > > > > > > > Anyway, I mentioned this idea only to understand the need for > > > > *optional* GUTS supports. Perhaps there is a cross SOC drivers that > > > > for some platforms depends on GUTS but on others it doesn't. > > > > > > > > Maybe that isn't case then!? > > > > > > Can you please answer this question!? > > > > > > According to the earlier versions of this patchset and from your > > > comments > > > [1], it *do* seems like the GUTS driver may be optional and thus stubs > > > could address this. > > > > > > Kind regards > > > Uffe > > > > > > [1] > > > http://www.spinics.net/lists/linux-mmc/msg34412.html > > > > [Lu Yangbo-B47093] Hi Scott and Uffe, > > In the earlier version, I'd like to use syscon support and only add > > 'syscon' compatible in the dts whose eSDHC needs to use it to get SVR. > > But I never thought this had caused so much discussion... :( > > Sorry, I understand your frustration but that's life sometimes. :-) > > To me, the syscon solution is more elegant... The syscon patch was terrible. It would have accessed a certain location in any node labelled "syscon" whether it was guts or not, in addition to the other complaints. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Scott Wood [mailto:scottwood@freescale.com] > Sent: Tuesday, December 29, 2015 3:10 AM > To: Ulf Hansson; Yangbo Lu > Cc: Lu Yangbo-B47093; linux-mmc; Xie Xiaobo-R63061; Leo li > Subject: Re: [v4, 5/6] mmc: kconfig: select FSL_GUTS for > MMC_SDHCI_OF_ESDHC > > On Mon, 2015-12-28 at 13:10 +0100, Ulf Hansson wrote: > > On 28 December 2015 at 11:26, Yangbo Lu <yangbo.lu@nxp.com> wrote: > > > > -----Original Message----- > > > > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > > > > Sent: Thursday, December 17, 2015 7:31 PM > > > > To: Scott Wood > > > > Cc: Lu Yangbo-B47093; linux-mmc; Xie Xiaobo-R63061; Leo li > > > > Subject: Re: [v4, 5/6] mmc: kconfig: select FSL_GUTS for > > > > MMC_SDHCI_OF_ESDHC > > > > > > > > [...] > > > > > > > > > > > > > > > > And I think stubs for reading SVR is quite a bad idea. It'll > > > > > > make the driver build but it will silently not be able to > > > > > > apply SVR-based > > > > workarounds. > > > > > > > > > > It doesn't have to be "silent", the driver can return an error > > > > > (and print error messages) from its ->probe() method, if the > > > > > calls to the GUTS driver fails. > > > > > > > > > > Anyway, I mentioned this idea only to understand the need for > > > > > *optional* GUTS supports. Perhaps there is a cross SOC drivers > > > > > that for some platforms depends on GUTS but on others it doesn't. > > > > > > > > > > Maybe that isn't case then!? > > > > > > > > Can you please answer this question!? > > > > > > > > According to the earlier versions of this patchset and from your > > > > comments [1], it *do* seems like the GUTS driver may be optional > > > > and thus stubs could address this. > > > > > > > > Kind regards > > > > Uffe > > > > > > > > [1] > > > > http://www.spinics.net/lists/linux-mmc/msg34412.html > > > > > > [Lu Yangbo-B47093] Hi Scott and Uffe, In the earlier version, I'd > > > like to use syscon support and only add 'syscon' compatible in the > > > dts whose eSDHC needs to use it to get SVR. > > > But I never thought this had caused so much discussion... :( > > > > Sorry, I understand your frustration but that's life sometimes. :-) > > > > To me, the syscon solution is more elegant... > > The syscon patch was terrible. It would have accessed a certain location > in any node labelled "syscon" whether it was guts or not, in addition to > the other complaints. > > -Scott [Lu Yangbo-B47093] As my understand, the syscon APIs would just check whether there is a 'syscon' compatible. If no, the APIs return. We still could maintain a list of compatibles for guts if using syscon. In my opinion, syscon and guts driver are just two method to get SVR. I agree with Uffe, because I think syscon is really designed for this situation and many arm platforms are using it. Of course, I still would like to try guts driver if you insist on it.
> -----Original Message----- > From: Scott Wood [mailto:scottwood@freescale.com] > Sent: Tuesday, December 29, 2015 2:48 AM > To: Ulf Hansson > Cc: Yangbo Lu; linux-mmc; X.Xie@freescale.com; Leo li > Subject: Re: [v4, 5/6] mmc: kconfig: select FSL_GUTS for > MMC_SDHCI_OF_ESDHC > > On Thu, 2015-12-17 at 12:30 +0100, Ulf Hansson wrote: > > [...] > > > > > > > > > > And I think stubs for reading SVR is quite a bad idea. It'll make > > > > the driver build but it will silently not be able to apply > > > > SVR-based workarounds. > > > > > > It doesn't have to be "silent", the driver can return an error (and > > > print error messages) from its ->probe() method, if the calls to the > > > GUTS driver fails. > > > > > > Anyway, I mentioned this idea only to understand the need for > > > *optional* GUTS supports. Perhaps there is a cross SOC drivers that > > > for some platforms depends on GUTS but on others it doesn't. > > > > > > Maybe that isn't case then!? > > > > Can you please answer this question!? > > > > According to the earlier versions of this patchset and from your > > comments [1], it *do* seems like the GUTS driver may be optional and > > thus stubs could address this. > > I'd rather it not be optional at build time for the reason I explained > above. > It would be too easy for users to accidentally not enable the guts > driver and miss the workaround. Even if an error is printed it's easy to > miss among all the boot spam -- and if there's any legitimate reason to > not have the driver enabled then why would that be an "error"? If the > guts driver fails to bind (e.g. running in a VM, or a platform the guts > driver doesn't recognize) that's another matter, but that should be > handled by an error check in the guts driver, not a build-time stub. > > -Scott > [Lu Yangbo-B47093] For the 'optional or not optional' problem, I think Scott's opinion is reasonable. If we use guts driver, 'select' could avoid missing the workaround. If we use syscon, select is better than 'depend on' since 'depend on' needs user self to find out the dependency. Anyway, we should continue the discussion to come to an agreement finally. I would very appreciate! Thanks.
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBVbGYgSGFuc3NvbiBbbWFpbHRv OnVsZi5oYW5zc29uQGxpbmFyby5vcmddDQo+IFNlbnQ6IE1vbmRheSwgRGVjZW1iZXIgMjgsIDIw MTUgODoxMCBQTQ0KPiBUbzogWWFuZ2JvIEx1DQo+IENjOiBTY290dCBXb29kOyBMdSBZYW5nYm8t QjQ3MDkzOyBsaW51eC1tbWM7IFhpZSBYaWFvYm8tUjYzMDYxOyBMZW8gbGkNCj4gU3ViamVjdDog UmU6IFt2NCwgNS82XSBtbWM6IGtjb25maWc6IHNlbGVjdCBGU0xfR1VUUyBmb3INCj4gTU1DX1NE SENJX09GX0VTREhDDQo+IA0KPiBPbiAyOCBEZWNlbWJlciAyMDE1IGF0IDExOjI2LCBZYW5nYm8g THUgPHlhbmdiby5sdUBueHAuY29tPiB3cm90ZToNCj4gPj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdl LS0tLS0NCj4gPj4gRnJvbTogVWxmIEhhbnNzb24gW21haWx0bzp1bGYuaGFuc3NvbkBsaW5hcm8u b3JnXQ0KPiA+PiBTZW50OiBUaHVyc2RheSwgRGVjZW1iZXIgMTcsIDIwMTUgNzozMSBQTQ0KPiA+ PiBUbzogU2NvdHQgV29vZA0KPiA+PiBDYzogTHUgWWFuZ2JvLUI0NzA5MzsgbGludXgtbW1jOyBY aWUgWGlhb2JvLVI2MzA2MTsgTGVvIGxpDQo+ID4+IFN1YmplY3Q6IFJlOiBbdjQsIDUvNl0gbW1j OiBrY29uZmlnOiBzZWxlY3QgRlNMX0dVVFMgZm9yDQo+ID4+IE1NQ19TREhDSV9PRl9FU0RIQw0K PiA+Pg0KPiA+PiBbLi4uXQ0KPiA+Pg0KPiA+PiA+Pg0KPiA+PiA+PiBBbmQgSSB0aGluayBzdHVi cyBmb3IgcmVhZGluZyBTVlIgaXMgcXVpdGUgYSBiYWQgaWRlYS4gIEl0J2xsIG1ha2UNCj4gPj4g Pj4gdGhlIGRyaXZlciBidWlsZCBidXQgaXQgd2lsbCBzaWxlbnRseSBub3QgYmUgYWJsZSB0byBh cHBseQ0KPiA+PiA+PiBTVlItYmFzZWQNCj4gPj4gd29ya2Fyb3VuZHMuDQo+ID4+ID4NCj4gPj4g PiBJdCBkb2Vzbid0IGhhdmUgdG8gYmUgInNpbGVudCIsIHRoZSBkcml2ZXIgY2FuIHJldHVybiBh biBlcnJvciAoYW5kDQo+ID4+ID4gcHJpbnQgZXJyb3IgbWVzc2FnZXMpIGZyb20gaXRzIC0+cHJv YmUoKSBtZXRob2QsIGlmIHRoZSBjYWxscyB0bw0KPiA+PiA+IHRoZSBHVVRTIGRyaXZlciBmYWls cy4NCj4gPj4gPg0KPiA+PiA+IEFueXdheSwgSSBtZW50aW9uZWQgdGhpcyBpZGVhIG9ubHkgdG8g dW5kZXJzdGFuZCB0aGUgbmVlZCBmb3INCj4gPj4gPiAqb3B0aW9uYWwqIEdVVFMgc3VwcG9ydHMu IFBlcmhhcHMgdGhlcmUgaXMgYSBjcm9zcyBTT0MgZHJpdmVycyB0aGF0DQo+ID4+ID4gZm9yIHNv bWUgcGxhdGZvcm1zIGRlcGVuZHMgb24gR1VUUyBidXQgb24gb3RoZXJzIGl0IGRvZXNuJ3QuDQo+ ID4+ID4NCj4gPj4gPiBNYXliZSB0aGF0IGlzbid0IGNhc2UgdGhlbiE/DQo+ID4+DQo+ID4+IENh biB5b3UgcGxlYXNlIGFuc3dlciB0aGlzIHF1ZXN0aW9uIT8NCj4gPj4NCj4gPj4gQWNjb3JkaW5n IHRvIHRoZSBlYXJsaWVyIHZlcnNpb25zIG9mIHRoaXMgcGF0Y2hzZXQgYW5kIGZyb20geW91cg0K PiA+PiBjb21tZW50cyBbMV0sIGl0ICpkbyogc2VlbXMgbGlrZSB0aGUgR1VUUyBkcml2ZXIgbWF5 IGJlIG9wdGlvbmFsIGFuZA0KPiA+PiB0aHVzIHN0dWJzIGNvdWxkIGFkZHJlc3MgdGhpcy4NCj4g Pj4NCj4gPj4gS2luZCByZWdhcmRzDQo+ID4+IFVmZmUNCj4gPj4NCj4gPj4gWzFdDQo+ID4+IGh0 dHA6Ly93d3cuc3Bpbmljcy5uZXQvbGlzdHMvbGludXgtbW1jL21zZzM0NDEyLmh0bWwNCj4gPg0K PiA+IFtMdSBZYW5nYm8tQjQ3MDkzXSBIaSBTY290dCBhbmQgVWZmZSwNCj4gPiBJbiB0aGUgZWFy bGllciB2ZXJzaW9uLCBJJ2QgbGlrZSB0byB1c2Ugc3lzY29uIHN1cHBvcnQgYW5kIG9ubHkgYWRk DQo+ICdzeXNjb24nIGNvbXBhdGlibGUgaW4gdGhlIGR0cyB3aG9zZSBlU0RIQyBuZWVkcyB0byB1 c2UgaXQgdG8gZ2V0IFNWUi4NCj4gPiBCdXQgSSBuZXZlciB0aG91Z2h0IHRoaXMgaGFkIGNhdXNl ZCBzbyBtdWNoIGRpc2N1c3Npb24uLi4gOigNCj4gDQo+IFNvcnJ5LCBJIHVuZGVyc3RhbmQgeW91 ciBmcnVzdHJhdGlvbiBidXQgdGhhdCdzIGxpZmUgc29tZXRpbWVzLiA6LSkNCj4gDQoNCltMdSBZ YW5nYm8tQjQ3MDkzXSBUaGFuayB5b3Ugc28gbXVjaCBmb3IgeW91ciB1bmRlcnN0YW5kaW5nIDop DQoNCj4gVG8gbWUsIHRoZSBzeXNjb24gc29sdXRpb24gaXMgbW9yZSBlbGVnYW50Li4uDQo+IA0K PiA+IENvdWxkIHdlIHJlYWNoIGFuIGFncmVlbWVudCBhYm91dCB0aGUgJ29wdGlvbmFsJyBvciBu b3QgJ29wdGlvbmFsJz8NCj4gDQo+IC4uLmJ1dCBJIGFtIGZpbmUgd2l0aCB0aGUgY3VycmVudCBh cHByb2FjaCBhcyB3ZWxsLCBhcyBsb25nIGFzIG15IHJlY2VudA0KPiBjb21tZW50cyBnZXRzIGFk ZHJlc3NlZC4gTGV0J3MgbWFrZSBpdCBvcHRpb25hbC4NCj4gDQo+IEFkZHJlc3MgU2NvdHQncyBh bmQgbXkgcmV2aWV3LWNvbW1lbnRzLCBnZXQgb3RoZXIgcGVvcGxlcyBhY2sgZm9yIHRoZQ0KPiBu b24tbW1jIHBhcnRzLCB0aGVuIEkgd2lsbCBoYXBwaWx5IHBpY2sgdXAgdGhlIHBhdGNoZXMuDQo+ IA0KDQpbTHUgWWFuZ2JvLUI0NzA5M10gVGhhbmtzIGEgbG90LiBJIHdvdWxkIGFkZCBvdGhlciBy ZXZpZXdlcnMgaW4gbmV4dCB2ZXJzaW9uLg0KIA0KPiBLaW5kIHJlZ2FyZHMNCj4gVWZmZQ0K -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Yangbo Lu > Sent: Wednesday, January 06, 2016 2:58 PM > To: Scott Wood; Ulf Hansson > Cc: Lu Yangbo-B47093; linux-mmc; Xie Xiaobo-R63061; Leo li > Subject: RE: [v4, 5/6] mmc: kconfig: select FSL_GUTS for > MMC_SDHCI_OF_ESDHC > > > -----Original Message----- > > From: Scott Wood [mailto:scottwood@freescale.com] > > Sent: Tuesday, December 29, 2015 3:10 AM > > To: Ulf Hansson; Yangbo Lu > > Cc: Lu Yangbo-B47093; linux-mmc; Xie Xiaobo-R63061; Leo li > > Subject: Re: [v4, 5/6] mmc: kconfig: select FSL_GUTS for > > MMC_SDHCI_OF_ESDHC > > > > On Mon, 2015-12-28 at 13:10 +0100, Ulf Hansson wrote: > > > On 28 December 2015 at 11:26, Yangbo Lu <yangbo.lu@nxp.com> wrote: > > > > > -----Original Message----- > > > > > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > > > > > Sent: Thursday, December 17, 2015 7:31 PM > > > > > To: Scott Wood > > > > > Cc: Lu Yangbo-B47093; linux-mmc; Xie Xiaobo-R63061; Leo li > > > > > Subject: Re: [v4, 5/6] mmc: kconfig: select FSL_GUTS for > > > > > MMC_SDHCI_OF_ESDHC > > > > > > > > > > [...] > > > > > > > > > > > > > > > > > > > And I think stubs for reading SVR is quite a bad idea. > > > > > > > It'll make the driver build but it will silently not be able > > > > > > > to apply SVR-based > > > > > workarounds. > > > > > > > > > > > > It doesn't have to be "silent", the driver can return an error > > > > > > (and print error messages) from its ->probe() method, if the > > > > > > calls to the GUTS driver fails. > > > > > > > > > > > > Anyway, I mentioned this idea only to understand the need for > > > > > > *optional* GUTS supports. Perhaps there is a cross SOC drivers > > > > > > that for some platforms depends on GUTS but on others it > doesn't. > > > > > > > > > > > > Maybe that isn't case then!? > > > > > > > > > > Can you please answer this question!? > > > > > > > > > > According to the earlier versions of this patchset and from your > > > > > comments [1], it *do* seems like the GUTS driver may be optional > > > > > and thus stubs could address this. > > > > > > > > > > Kind regards > > > > > Uffe > > > > > > > > > > [1] > > > > > http://www.spinics.net/lists/linux-mmc/msg34412.html > > > > > > > > [Lu Yangbo-B47093] Hi Scott and Uffe, In the earlier version, I'd > > > > like to use syscon support and only add 'syscon' compatible in the > > > > dts whose eSDHC needs to use it to get SVR. > > > > But I never thought this had caused so much discussion... :( > > > > > > Sorry, I understand your frustration but that's life sometimes. :-) > > > > > > To me, the syscon solution is more elegant... > > > > The syscon patch was terrible. It would have accessed a certain > > location in any node labelled "syscon" whether it was guts or not, in > > addition to the other complaints. > > > > -Scott > > [Lu Yangbo-B47093] As my understand, the syscon APIs would just check > whether there is a 'syscon' compatible. > If no, the APIs return. We still could maintain a list of compatibles for > guts if using syscon. [Lu Yangbo-B47093] It's ok to use a compatible name that is not 'syscon' as the parameter of the APIs. The node needs just to contain 'syscon'. > In my opinion, syscon and guts driver are just two method to get SVR. > > I agree with Uffe, because I think syscon is really designed for this > situation and many arm platforms are using it. > Of course, I still would like to try guts driver if you insist on it. > > > > >
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBZYW5nYm8gTHUNCj4gU2VudDog V2VkbmVzZGF5LCBKYW51YXJ5IDA2LCAyMDE2IDM6MTggUE0NCj4gVG86IFNjb3R0IFdvb2Q7IFVs ZiBIYW5zc29uDQo+IENjOiBZYW5nYm8gTHU7IGxpbnV4LW1tYzsgWC5YaWVAZnJlZXNjYWxlLmNv bTsgTGVvIGxpDQo+IFN1YmplY3Q6IFJFOiBbdjQsIDUvNl0gbW1jOiBrY29uZmlnOiBzZWxlY3Qg RlNMX0dVVFMgZm9yDQo+IE1NQ19TREhDSV9PRl9FU0RIQw0KPiANCj4gPiAtLS0tLU9yaWdpbmFs IE1lc3NhZ2UtLS0tLQ0KPiA+IEZyb206IFNjb3R0IFdvb2QgW21haWx0bzpzY290dHdvb2RAZnJl ZXNjYWxlLmNvbV0NCj4gPiBTZW50OiBUdWVzZGF5LCBEZWNlbWJlciAyOSwgMjAxNSAyOjQ4IEFN DQo+ID4gVG86IFVsZiBIYW5zc29uDQo+ID4gQ2M6IFlhbmdibyBMdTsgbGludXgtbW1jOyBYLlhp ZUBmcmVlc2NhbGUuY29tOyBMZW8gbGkNCj4gPiBTdWJqZWN0OiBSZTogW3Y0LCA1LzZdIG1tYzog a2NvbmZpZzogc2VsZWN0IEZTTF9HVVRTIGZvcg0KPiA+IE1NQ19TREhDSV9PRl9FU0RIQw0KPiA+ DQo+ID4gT24gVGh1LCAyMDE1LTEyLTE3IGF0IDEyOjMwICswMTAwLCBVbGYgSGFuc3NvbiB3cm90 ZToNCj4gPiA+IFsuLi5dDQo+ID4gPg0KPiA+ID4gPiA+DQo+ID4gPiA+ID4gQW5kIEkgdGhpbmsg c3R1YnMgZm9yIHJlYWRpbmcgU1ZSIGlzIHF1aXRlIGEgYmFkIGlkZWEuICBJdCdsbA0KPiA+ID4g PiA+IG1ha2UgdGhlIGRyaXZlciBidWlsZCBidXQgaXQgd2lsbCBzaWxlbnRseSBub3QgYmUgYWJs ZSB0byBhcHBseQ0KPiA+ID4gPiA+IFNWUi1iYXNlZCB3b3JrYXJvdW5kcy4NCj4gPiA+ID4NCj4g PiA+ID4gSXQgZG9lc24ndCBoYXZlIHRvIGJlICJzaWxlbnQiLCB0aGUgZHJpdmVyIGNhbiByZXR1 cm4gYW4gZXJyb3INCj4gPiA+ID4gKGFuZCBwcmludCBlcnJvciBtZXNzYWdlcykgZnJvbSBpdHMg LT5wcm9iZSgpIG1ldGhvZCwgaWYgdGhlIGNhbGxzDQo+ID4gPiA+IHRvIHRoZSBHVVRTIGRyaXZl ciBmYWlscy4NCj4gPiA+ID4NCj4gPiA+ID4gQW55d2F5LCBJIG1lbnRpb25lZCB0aGlzIGlkZWEg b25seSB0byB1bmRlcnN0YW5kIHRoZSBuZWVkIGZvcg0KPiA+ID4gPiAqb3B0aW9uYWwqIEdVVFMg c3VwcG9ydHMuIFBlcmhhcHMgdGhlcmUgaXMgYSBjcm9zcyBTT0MgZHJpdmVycw0KPiA+ID4gPiB0 aGF0IGZvciBzb21lIHBsYXRmb3JtcyBkZXBlbmRzIG9uIEdVVFMgYnV0IG9uIG90aGVycyBpdCBk b2Vzbid0Lg0KPiA+ID4gPg0KPiA+ID4gPiBNYXliZSB0aGF0IGlzbid0IGNhc2UgdGhlbiE/DQo+ ID4gPg0KPiA+ID4gQ2FuIHlvdSBwbGVhc2UgYW5zd2VyIHRoaXMgcXVlc3Rpb24hPw0KPiA+ID4N Cj4gPiA+IEFjY29yZGluZyB0byB0aGUgZWFybGllciB2ZXJzaW9ucyBvZiB0aGlzIHBhdGNoc2V0 IGFuZCBmcm9tIHlvdXINCj4gPiA+IGNvbW1lbnRzIFsxXSwgaXQgKmRvKiBzZWVtcyBsaWtlIHRo ZSBHVVRTIGRyaXZlciBtYXkgYmUgb3B0aW9uYWwgYW5kDQo+ID4gPiB0aHVzIHN0dWJzIGNvdWxk IGFkZHJlc3MgdGhpcy4NCj4gPg0KPiA+IEknZCByYXRoZXIgaXQgbm90IGJlIG9wdGlvbmFsIGF0 IGJ1aWxkIHRpbWUgZm9yIHRoZSByZWFzb24gSSBleHBsYWluZWQNCj4gPiBhYm92ZS4NCj4gPiAg SXQgd291bGQgYmUgdG9vIGVhc3kgZm9yIHVzZXJzIHRvIGFjY2lkZW50YWxseSBub3QgZW5hYmxl IHRoZSBndXRzDQo+ID4gZHJpdmVyIGFuZCBtaXNzIHRoZSB3b3JrYXJvdW5kLiAgRXZlbiBpZiBh biBlcnJvciBpcyBwcmludGVkIGl0J3MgZWFzeQ0KPiA+IHRvIG1pc3MgYW1vbmcgYWxsIHRoZSBi b290IHNwYW0gLS0gYW5kIGlmIHRoZXJlJ3MgYW55IGxlZ2l0aW1hdGUNCj4gPiByZWFzb24gdG8g bm90IGhhdmUgdGhlIGRyaXZlciBlbmFibGVkIHRoZW4gd2h5IHdvdWxkIHRoYXQgYmUgYW4NCj4g PiAiZXJyb3IiPyAgSWYgdGhlIGd1dHMgZHJpdmVyIGZhaWxzIHRvIGJpbmQgKGUuZy4gcnVubmlu ZyBpbiBhIFZNLCBvciBhDQo+ID4gcGxhdGZvcm0gdGhlIGd1dHMgZHJpdmVyIGRvZXNuJ3QgcmVj b2duaXplKSB0aGF0J3MgYW5vdGhlciBtYXR0ZXIsIGJ1dA0KPiA+IHRoYXQgc2hvdWxkIGJlIGhh bmRsZWQgYnkgYW4gZXJyb3IgY2hlY2sgaW4gdGhlIGd1dHMgZHJpdmVyLCBub3QgYQ0KPiBidWls ZC10aW1lIHN0dWIuDQo+ID4NCj4gPiAtU2NvdHQNCj4gPg0KPiANCj4gW0x1IFlhbmdiby1CNDcw OTNdIEZvciB0aGUgJ29wdGlvbmFsIG9yIG5vdCBvcHRpb25hbCcgcHJvYmxlbSwgSSB0aGluaw0K PiBTY290dCdzIG9waW5pb24gaXMgcmVhc29uYWJsZS4NCj4gSWYgd2UgdXNlIGd1dHMgZHJpdmVy LCAnc2VsZWN0JyBjb3VsZCBhdm9pZCBtaXNzaW5nIHRoZSB3b3JrYXJvdW5kLg0KPiBJZiB3ZSB1 c2Ugc3lzY29uLCBzZWxlY3QgaXMgYmV0dGVyIHRoYW4gJ2RlcGVuZCBvbicgc2luY2UgJ2RlcGVu ZCBvbicNCj4gbmVlZHMgdXNlciBzZWxmIHRvIGZpbmQgb3V0IHRoZSBkZXBlbmRlbmN5Lg0KPiAN Cj4gQW55d2F5LCB3ZSBzaG91bGQgY29udGludWUgdGhlIGRpc2N1c3Npb24gdG8gY29tZSB0byBh biBhZ3JlZW1lbnQgZmluYWxseS4NCj4gSSB3b3VsZCB2ZXJ5IGFwcHJlY2lhdGUhDQo+IA0KPiBU aGFua3MuDQo+IA0KPiANCltMdSBZYW5nYm8tQjQ3MDkzXSBIaSBVZmZlLCBkbyB5b3UgaGF2ZSBh bnkgY29tbWVudHM/IDopDQoNCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2016-01-08 at 06:17 +0000, Yangbo Lu wrote: > > -----Original Message----- > > From: Scott Wood > > Sent: Friday, January 08, 2016 6:24 AM > > To: Yangbo Lu; Scott Wood; Ulf Hansson > > Cc: Lu Y.B.; linux-mmc; Xiaobo Xie; Li Leo > > Subject: Re: [v4, 5/6] mmc: kconfig: select FSL_GUTS for > > MMC_SDHCI_OF_ESDHC > > > > On 01/06/2016 12:58 AM, Yangbo Lu wrote: > > > > -----Original Message----- > > > > From: Scott Wood [mailto:scottwood@freescale.com] > > > > Sent: Tuesday, December 29, 2015 3:10 AM > > > > To: Ulf Hansson; Yangbo Lu > > > > Cc: Lu Yangbo-B47093; linux-mmc; Xie Xiaobo-R63061; Leo li > > > > Subject: Re: [v4, 5/6] mmc: kconfig: select FSL_GUTS for > > > > MMC_SDHCI_OF_ESDHC > > > > > > > > On Mon, 2015-12-28 at 13:10 +0100, Ulf Hansson wrote: > > > > > On 28 December 2015 at 11:26, Yangbo Lu <yangbo.lu@nxp.com> wrote: > > > > > > > -----Original Message----- > > > > > > > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > > > > > > > Sent: Thursday, December 17, 2015 7:31 PM > > > > > > > To: Scott Wood > > > > > > > Cc: Lu Yangbo-B47093; linux-mmc; Xie Xiaobo-R63061; Leo li > > > > > > > Subject: Re: [v4, 5/6] mmc: kconfig: select FSL_GUTS for > > > > > > > MMC_SDHCI_OF_ESDHC > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > > > > > > > > > > > > And I think stubs for reading SVR is quite a bad idea. > > > > > > > > > It'll > > > > > > > > > make the driver build but it will silently not be able to > > > > > > > > > apply > > > > > > > > > SVR-based > > > > > > > workarounds. > > > > > > > > > > > > > > > > It doesn't have to be "silent", the driver can return an error > > > > > > > > (and print error messages) from its ->probe() method, if the > > > > > > > > calls to the GUTS driver fails. > > > > > > > > > > > > > > > > Anyway, I mentioned this idea only to understand the need for > > > > > > > > *optional* GUTS supports. Perhaps there is a cross SOC drivers > > > > > > > > that for some platforms depends on GUTS but on others it > > > > > > > > doesn't. > > > > > > > > > > > > > > > > Maybe that isn't case then!? > > > > > > > > > > > > > > Can you please answer this question!? > > > > > > > > > > > > > > According to the earlier versions of this patchset and from your > > > > > > > comments [1], it *do* seems like the GUTS driver may be optional > > > > > > > and thus stubs could address this. > > > > > > > > > > > > > > Kind regards > > > > > > > Uffe > > > > > > > > > > > > > > [1] > > > > > > > http://www.spinics.net/lists/linux-mmc/msg34412.html > > > > > > > > > > > > [Lu Yangbo-B47093] Hi Scott and Uffe, In the earlier version, I'd > > > > > > like to use syscon support and only add 'syscon' compatible in the > > > > > > dts whose eSDHC needs to use it to get SVR. > > > > > > But I never thought this had caused so much discussion... :( > > > > > > > > > > Sorry, I understand your frustration but that's life sometimes. :-) > > > > > > > > > > To me, the syscon solution is more elegant... > > > > > > > > The syscon patch was terrible. It would have accessed a certain > > > > location in any node labelled "syscon" whether it was guts or not, in > > > > addition to the other complaints. > > > > > > > > -Scott > > > > > > [Lu Yangbo-B47093] As my understand, the syscon APIs would just check > > whether there is a 'syscon' compatible. > > > If no, the APIs return. We still could maintain a list of compatibles > > for guts if using syscon. > > > > Where would that list of compatibles go? Duplicated in every driver that > > needs something from guts? Why decentralize the implementation? > > > > > In my opinion, syscon and guts driver are just two method to get SVR. > > > > The guts driver could eventually handle more than just SVR, and it could > > handle SVR better (e.g. on PPC fall back to reading the SPR if guts is > > unavailable). > > > > -Scott > > [Lu Yangbo-B47093] Ok... It seems no proper place to maintain the guts list > presently... > I also have had a question about the guts driver. > Could you suggest where to register the guts driver, to make it registered > for all QorIQ platforms and avoid unavailability to call it? In an initcall, just like other normal drivers. Maybe subsys_initcall. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> [Lu Yangbo-B47093] For the 'optional or not optional' problem, I think Scott's opinion is reasonable. > If we use guts driver, 'select' could avoid missing the workaround. > If we use syscon, select is better than 'depend on' since 'depend on' needs user self to find out the dependency. > > Anyway, we should continue the discussion to come to an agreement finally. > I would very appreciate! Please, go ahead and use the "select guts driver" option, as you both seems to feel that strong about it. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index 1526b8a..df57c14 100644 --- a/drivers/mmc/host/Kconfig +++ b/drivers/mmc/host/Kconfig @@ -142,6 +142,8 @@ config MMC_SDHCI_OF_ESDHC depends on MMC_SDHCI_PLTFM depends on PPC || ARCH_MXC || ARCH_LAYERSCAPE select MMC_SDHCI_IO_ACCESSORS + select SOC_FSL + select FSL_GUTS help This selects the Freescale eSDHC controller support.
The sdhci-of-esdhc driver needs the GUTS driver support to access the global utilities registers for SVR value. So we select FSL_GUTS for MMC_SDHCI_OF_ESDHC here. Signed-off-by: Yangbo Lu <yangbo.lu@freescale.com> --- Changes for v2: - None Changes for v3: - None Changes for v4: - Added this patch --- drivers/mmc/host/Kconfig | 2 ++ 1 file changed, 2 insertions(+)