[PATCH] mmc: Add hardware dependencies for sdhci-pxav3 and sdhci-pxav2

[PATCH] mmc: Add hardware dependencies for sdhci-pxav3 and sdhci-pxav2

[Jean Delvare]

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.

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

Re: [PATCH] mmc: Add hardware dependencies for sdhci-pxav3 and sdhci-pxav2

[Ulf Hansson]

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

Re: [PATCH] mmc: Add hardware dependencies for sdhci-pxav3 and sdhci-pxav2

[Jean Delvare]

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

Re: [PATCH] mmc: Add hardware dependencies for sdhci-pxav3 and sdhci-pxav2

[Ulf Hansson]

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
>

Re: [PATCH] mmc: Add hardware dependencies for sdhci-pxav3 and sdhci-pxav2

[Jean Delvare]

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.

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] mmc: Add hardware dependencies for sdhci-pxav3 and sdhci-pxav2

[Ulf Hansson]

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

Re: [PATCH] mmc: Add hardware dependencies for sdhci-pxav3 and sdhci-pxav2

[Jean Delvare]

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.

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] mmc: Add hardware dependencies for sdhci-pxav3 and sdhci-pxav2

[Ulf Hansson]

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

Re: [PATCH] mmc: Add hardware dependencies for sdhci-pxav3 and sdhci-pxav2

[Jean Delvare]

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!

-- 
Jean Delvare
SUSE L3 Support