| Message ID | 20150126112328.318da5e7@endymion.delvare (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 26 January 2015 at 11:23, Jean Delvare <jdelvare@suse.de> wrote: > I seem to understand that the sdhci-pxav3 and sdhci-pxav2 drivers are > only needed on the MMP architecture. So add a hardware dependency on > ARCH_MMP, so that other users don't get to build useless drivers. I would rather see the default option to be N. Thus those configurations that needs this driver will have to select it. Kind regards Uffe > > Signed-off-by: Jean Delvare <jdelvare@suse.de> > Cc: Chris Ball <chris@printf.net> > Cc: Ulf Hansson <ulf.hansson@linaro.org> > Cc: Eric Miao <eric.y.miao@gmail.com> > Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com> > --- > This patch was already sent on: > * 2014-04-23 > * 2014-06-16 > > drivers/mmc/host/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > > --- linux-3.19-rc6.orig/drivers/mmc/host/Kconfig 2015-01-26 10:30:56.472182636 +0100 > +++ linux-3.19-rc6/drivers/mmc/host/Kconfig 2015-01-26 11:13:33.669863314 +0100 > @@ -228,6 +228,7 @@ config MMC_SDHCI_PXAV3 > tristate "Marvell MMP2 SD Host Controller support (PXAV3)" > depends on CLKDEV_LOOKUP > depends on MMC_SDHCI_PLTFM > + depends on ARCH_MMP || COMPILE_TEST > default CPU_MMP2 > help > This selects the Marvell(R) PXAV3 SD Host Controller. > @@ -240,6 +241,7 @@ config MMC_SDHCI_PXAV2 > tristate "Marvell PXA9XX SD Host Controller support (PXAV2)" > depends on CLKDEV_LOOKUP > depends on MMC_SDHCI_PLTFM > + depends on ARCH_MMP || COMPILE_TEST > default CPU_PXA910 > help > This selects the Marvell(R) PXAV2 SD Host Controller. > > > -- > Jean Delvare > SUSE L3 Support -- 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
Hi Ulf, Le Tuesday 27 January 2015 à 15:06 +0100, Ulf Hansson a écrit : > On 26 January 2015 at 11:23, Jean Delvare <jdelvare@suse.de> wrote: > > I seem to understand that the sdhci-pxav3 and sdhci-pxav2 drivers are > > only needed on the MMP architecture. So add a hardware dependency on > > ARCH_MMP, so that other users don't get to build useless drivers. > > I would rather see the default option to be N. > Thus those configurations that needs this driver will have to select it. This is a different question. The purpose of my patch is that people configuring kernels for systems which just can't have these controllers, are not asked about this driver at all. Changing the default to N would not achieve that. That being said, feel free to change the default to N if you want, but to me (who knows nothing about MMP architecture and not much about MMC) the current default values look sane. Thanks, Jean > > Signed-off-by: Jean Delvare <jdelvare@suse.de> > > Cc: Chris Ball <chris@printf.net> > > Cc: Ulf Hansson <ulf.hansson@linaro.org> > > Cc: Eric Miao <eric.y.miao@gmail.com> > > Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com> > > --- > > This patch was already sent on: > > * 2014-04-23 > > * 2014-06-16 > > > > drivers/mmc/host/Kconfig | 2 ++ > > 1 file changed, 2 insertions(+) > > > > --- linux-3.19-rc6.orig/drivers/mmc/host/Kconfig 2015-01-26 10:30:56.472182636 +0100 > > +++ linux-3.19-rc6/drivers/mmc/host/Kconfig 2015-01-26 11:13:33.669863314 +0100 > > @@ -228,6 +228,7 @@ config MMC_SDHCI_PXAV3 > > tristate "Marvell MMP2 SD Host Controller support (PXAV3)" > > depends on CLKDEV_LOOKUP > > depends on MMC_SDHCI_PLTFM > > + depends on ARCH_MMP || COMPILE_TEST > > default CPU_MMP2 > > help > > This selects the Marvell(R) PXAV3 SD Host Controller. > > @@ -240,6 +241,7 @@ config MMC_SDHCI_PXAV2 > > tristate "Marvell PXA9XX SD Host Controller support (PXAV2)" > > depends on CLKDEV_LOOKUP > > depends on MMC_SDHCI_PLTFM > > + depends on ARCH_MMP || COMPILE_TEST > > default CPU_PXA910 > > help > > This selects the Marvell(R) PXAV2 SD Host Controller. > > > > > > -- > > Jean Delvare > > SUSE L3 Support -- 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 27 January 2015 at 15:34, Jean Delvare <jdelvare@suse.de> wrote: > Hi Ulf, > > Le Tuesday 27 January 2015 à 15:06 +0100, Ulf Hansson a écrit : >> On 26 January 2015 at 11:23, Jean Delvare <jdelvare@suse.de> wrote: >> > I seem to understand that the sdhci-pxav3 and sdhci-pxav2 drivers are >> > only needed on the MMP architecture. So add a hardware dependency on >> > ARCH_MMP, so that other users don't get to build useless drivers. >> >> I would rather see the default option to be N. >> Thus those configurations that needs this driver will have to select it. > > This is a different question. The purpose of my patch is that people > configuring kernels for systems which just can't have these controllers, > are not asked about this driver at all. Changing the default to N would > not achieve that. For those SOC that want these drivers, they should be able to select them from their defconfigs. So it will be an opt-in instead of opt-out policy, which I prefer. It also follows the other Kconfig options for mmc drivers. Kind regards Uffe > > That being said, feel free to change the default to N if you want, but > to me (who knows nothing about MMP architecture and not much about MMC) > the current default values look sane. > > Thanks, > Jean > >> > Signed-off-by: Jean Delvare <jdelvare@suse.de> >> > Cc: Chris Ball <chris@printf.net> >> > Cc: Ulf Hansson <ulf.hansson@linaro.org> >> > Cc: Eric Miao <eric.y.miao@gmail.com> >> > Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com> >> > --- >> > This patch was already sent on: >> > * 2014-04-23 >> > * 2014-06-16 >> > >> > drivers/mmc/host/Kconfig | 2 ++ >> > 1 file changed, 2 insertions(+) >> > >> > --- linux-3.19-rc6.orig/drivers/mmc/host/Kconfig 2015-01-26 10:30:56.472182636 +0100 >> > +++ linux-3.19-rc6/drivers/mmc/host/Kconfig 2015-01-26 11:13:33.669863314 +0100 >> > @@ -228,6 +228,7 @@ config MMC_SDHCI_PXAV3 >> > tristate "Marvell MMP2 SD Host Controller support (PXAV3)" >> > depends on CLKDEV_LOOKUP >> > depends on MMC_SDHCI_PLTFM >> > + depends on ARCH_MMP || COMPILE_TEST >> > default CPU_MMP2 >> > help >> > This selects the Marvell(R) PXAV3 SD Host Controller. >> > @@ -240,6 +241,7 @@ config MMC_SDHCI_PXAV2 >> > tristate "Marvell PXA9XX SD Host Controller support (PXAV2)" >> > depends on CLKDEV_LOOKUP >> > depends on MMC_SDHCI_PLTFM >> > + depends on ARCH_MMP || COMPILE_TEST >> > default CPU_PXA910 >> > help >> > This selects the Marvell(R) PXAV2 SD Host Controller. >> > >> > >> > -- >> > Jean Delvare >> > SUSE L3 Support > -- 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
Hi Ulf, On Wed, 28 Jan 2015 15:04:24 +0100, Ulf Hansson wrote: > On 27 January 2015 at 15:34, Jean Delvare <jdelvare@suse.de> wrote: > > Hi Ulf, > > > > Le Tuesday 27 January 2015 à 15:06 +0100, Ulf Hansson a écrit : > >> On 26 January 2015 at 11:23, Jean Delvare <jdelvare@suse.de> wrote: > >> > I seem to understand that the sdhci-pxav3 and sdhci-pxav2 drivers are > >> > only needed on the MMP architecture. So add a hardware dependency on > >> > ARCH_MMP, so that other users don't get to build useless drivers. > >> > >> I would rather see the default option to be N. > >> Thus those configurations that needs this driver will have to select it. > > > > This is a different question. The purpose of my patch is that people > > configuring kernels for systems which just can't have these controllers, > > are not asked about this driver at all. Changing the default to N would > > not achieve that. > > For those SOC that want these drivers, they should be able to select > them from their defconfigs. So it will be an opt-in instead of opt-out > policy, which I prefer. It also follows the other Kconfig options for > mmc drivers. As you wish. But that change would be a separate patch going on top of mine, right? I'm not sure I understand what you expect from me at this point, please clarify.
On 29 January 2015 at 15:17, Jean Delvare <jdelvare@suse.de> wrote: > Hi Ulf, > > On Wed, 28 Jan 2015 15:04:24 +0100, Ulf Hansson wrote: >> On 27 January 2015 at 15:34, Jean Delvare <jdelvare@suse.de> wrote: >> > Hi Ulf, >> > >> > Le Tuesday 27 January 2015 à 15:06 +0100, Ulf Hansson a écrit : >> >> On 26 January 2015 at 11:23, Jean Delvare <jdelvare@suse.de> wrote: >> >> > I seem to understand that the sdhci-pxav3 and sdhci-pxav2 drivers are >> >> > only needed on the MMP architecture. So add a hardware dependency on >> >> > ARCH_MMP, so that other users don't get to build useless drivers. >> >> >> >> I would rather see the default option to be N. >> >> Thus those configurations that needs this driver will have to select it. >> > >> > This is a different question. The purpose of my patch is that people >> > configuring kernels for systems which just can't have these controllers, >> > are not asked about this driver at all. Changing the default to N would >> > not achieve that. >> >> For those SOC that want these drivers, they should be able to select >> them from their defconfigs. So it will be an opt-in instead of opt-out >> policy, which I prefer. It also follows the other Kconfig options for >> mmc drivers. > > As you wish. But that change would be a separate patch going on top of > mine, right? I'm not sure I understand what you expect from me at this > point, please clarify. Sorry for being unclear. I don't like $subject patch. Send a new one, removing the following lines: default CPU_MMP2 default CPU_PXA910 Then you send another patch(es) to the respective SOC maintainer, updating the defonfig(s) selecting MMC_SDHCI_PXAV3|PXAV2, when appropriate. Would that work? 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
Hi Ulf, On Thu, 29 Jan 2015 16:01:48 +0100, Ulf Hansson wrote: > On 29 January 2015 at 15:17, Jean Delvare <jdelvare@suse.de> wrote: > > On Wed, 28 Jan 2015 15:04:24 +0100, Ulf Hansson wrote: > >> For those SOC that want these drivers, they should be able to select > >> them from their defconfigs. So it will be an opt-in instead of opt-out > >> policy, which I prefer. It also follows the other Kconfig options for > >> mmc drivers. > > > > As you wish. But that change would be a separate patch going on top of > > mine, right? I'm not sure I understand what you expect from me at this > > point, please clarify. > > Sorry for being unclear. I don't like $subject patch. > > Send a new one, removing the following lines: > default CPU_MMP2 > default CPU_PXA910 > > Then you send another patch(es) to the respective SOC maintainer, > updating the defonfig(s) selecting MMC_SDHCI_PXAV3|PXAV2, when > appropriate. > > Would that work? Not really, I'm afraid. My proposed change affects users of non-embedded systems, or more generally everyone not on ARCH_MMP. I want to make their life easier by hiding options which are not relevant to them. I have sent several dozen of such patches in the past for various drivers, see for example the latest ones: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=441fb7684782be3553c67dc04defcf304b999bba http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c03842d89b769db44be5cb0b1ebb384ccfa25f7f http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=84c3a8f6eadb2bedfba10f62da0328d8533c8f25 Also note that the MMC subsystem already has examples of this, check MMC_OMAP_HS, MMC_SDHCI_MSM, MMC_SDHI, MMC_DW and MMC_SH_MMCIF in drivers/mmc/host/Kconfig. I'm just doing more of the same, nothing new. The change you want, OTOH, would affect exclusively the ARCH_MMP users (of which I am not.) It is essentially unrelated with what I was originally talking about, except for the fact that it touches the same Kconfig entries. I have no idea if your proposal is a good idea, I am not dealing with embedded systems, and I have no idea who are the maintainers of the affected SOCs. This is simply not my area. So basically you are rejecting my proposal without a reason, and then you ask me to do an unrelated work instead. This is not fair, sorry. Don't get me wrong, I'm always ready to do some more work than I originally intended if that's what it takes to get my patches merged. I value code review and I welcome constructive criticism. But this time your request is not reasonable.
On 30 January 2015 at 09:29, Jean Delvare <jdelvare@suse.de> wrote: > Hi Ulf, > > On Thu, 29 Jan 2015 16:01:48 +0100, Ulf Hansson wrote: >> On 29 January 2015 at 15:17, Jean Delvare <jdelvare@suse.de> wrote: >> > On Wed, 28 Jan 2015 15:04:24 +0100, Ulf Hansson wrote: >> >> For those SOC that want these drivers, they should be able to select >> >> them from their defconfigs. So it will be an opt-in instead of opt-out >> >> policy, which I prefer. It also follows the other Kconfig options for >> >> mmc drivers. >> > >> > As you wish. But that change would be a separate patch going on top of >> > mine, right? I'm not sure I understand what you expect from me at this >> > point, please clarify. >> >> Sorry for being unclear. I don't like $subject patch. >> >> Send a new one, removing the following lines: >> default CPU_MMP2 >> default CPU_PXA910 >> >> Then you send another patch(es) to the respective SOC maintainer, >> updating the defonfig(s) selecting MMC_SDHCI_PXAV3|PXAV2, when >> appropriate. >> >> Would that work? > > Not really, I'm afraid. > > My proposed change affects users of non-embedded systems, or more > generally everyone not on ARCH_MMP. I want to make their life easier by > hiding options which are not relevant to them. I have sent several > dozen of such patches in the past for various drivers, see for example > the latest ones: > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=441fb7684782be3553c67dc04defcf304b999bba > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c03842d89b769db44be5cb0b1ebb384ccfa25f7f > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=84c3a8f6eadb2bedfba10f62da0328d8533c8f25 > Also note that the MMC subsystem already has examples of this, check > MMC_OMAP_HS, MMC_SDHCI_MSM, MMC_SDHI, MMC_DW and MMC_SH_MMCIF in > drivers/mmc/host/Kconfig. I'm just doing more of the same, nothing new. > > The change you want, OTOH, would affect exclusively the ARCH_MMP users > (of which I am not.) It is essentially unrelated with what I was > originally talking about, except for the fact that it touches the same > Kconfig entries. I have no idea if your proposal is a good idea, I am > not dealing with embedded systems, and I have no idea who are the > maintainers of the affected SOCs. This is simply not my area. > > So basically you are rejecting my proposal without a reason, and then > you ask me to do an unrelated work instead. This is not fair, sorry. > > Don't get me wrong, I'm always ready to do some more work than I > originally intended if that's what it takes to get my patches merged. I > value code review and I welcome constructive criticism. But this time > your request is not reasonable. I did a little more of investigation of what's good practice/policy around your suggested patch. I have changed my mind, I am going to accept your patch as is. Sorry for all the noise and thanks for a good discussion. Applied for next. Thanks! 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 Fri, 30 Jan 2015 11:43:17 +0100, Ulf Hansson wrote: > I did a little more of investigation of what's good practice/policy > around your suggested patch. I have changed my mind, I am going to > accept your patch as is. > > Sorry for all the noise and thanks for a good discussion. > > Applied for next. Thanks! Great, thank you very much!
--- linux-3.19-rc6.orig/drivers/mmc/host/Kconfig 2015-01-26 10:30:56.472182636 +0100 +++ linux-3.19-rc6/drivers/mmc/host/Kconfig 2015-01-26 11:13:33.669863314 +0100 @@ -228,6 +228,7 @@ config MMC_SDHCI_PXAV3 tristate "Marvell MMP2 SD Host Controller support (PXAV3)" depends on CLKDEV_LOOKUP depends on MMC_SDHCI_PLTFM + depends on ARCH_MMP || COMPILE_TEST default CPU_MMP2 help This selects the Marvell(R) PXAV3 SD Host Controller. @@ -240,6 +241,7 @@ config MMC_SDHCI_PXAV2 tristate "Marvell PXA9XX SD Host Controller support (PXAV2)" depends on CLKDEV_LOOKUP depends on MMC_SDHCI_PLTFM + depends on ARCH_MMP || COMPILE_TEST default CPU_PXA910 help This selects the Marvell(R) PXAV2 SD Host Controller.