Re: [PATCH v4 1/2] pwm: lpss: properly split driver to parts

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1805 bytes --]

On Mon, Aug 25, 2014 at 11:11:17AM +0300, Andy Shevchenko wrote:
> On Fri, 2014-08-22 at 13:13 +0200, Thierry Reding wrote:
> > On Tue, Aug 19, 2014 at 07:17:35PM +0300, Andy Shevchenko wrote:
> > [...]
> > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> > [...]
> > > +config PWM_LPSS_PCI
> > > +	tristate "Intel LPSS PWM PCI driver"
> > > +	depends on PCI
> > > +	select PWM_LPSS
> > [...]
> > > +config PWM_LPSS_PLATFORM
> > > +	tristate "Intel LPSS PWM platform driver"
> > > +	depends on ACPI
> > > +	select PWM_LPSS
> > 
> > I changed both of the above select PWM_LPSS to depends on PWM_LPSS
> > because that makes them show up in a more meaningful way in menuconfig
> > and otherwise it looks weird if the PWM_LPSS shows up as automatically
> > selected without being useful in itself. This way the PWM_LPSS is sort
> > of a common core that PWM_LPSS_PLATFORM and PWM_LPSS_PCI use.
> > 
> > An alternative that's commonly used for this would be to hide PWM_LPSS
> > from users and keep the select within PWM_LPSS_PLATFORM and
> > PWM_LPSS_PCI. That would be okay with me too.
> > 
> > Let me know if you have any objections to this change.
> 
> 
> Didn't notice your message before.
> Regarding to this one I think the way to hide PWM_LPSS from user would
> be preferred, though I'm okay if it keeps visible.
> 
> In addition to compile error you get from Stephen, thanks for quick fix,
> though I think we have to make PWM_LPSS dependent to X86 for now. I
> don't believe we will have same IP on other architectures.

I does seem to build fine for other architectures, so I'm fine with
keeping it available for non-x86. But I don't have a strong opinion
either way, so if you'd rather make it x86-specific, feel free to
send a patch.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]