linux-kernel - Re: [PATCHv3 1/4] phy: phy-core: Make GENERIC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150529150413.GB2026@saruman.tx.rr.com>
Date:	Fri, 29 May 2015 10:04:13 -0500
From:	Felipe Balbi <balbi@...com>
To:	Kishon Vijay Abraham I <kishon@...com>
CC:	<balbi@...com>, Arun Ramamurthy <arun.ramamurthy@...adcom.com>,
	Jonathan Corbet <corbet@....net>, Tejun Heo <tj@...nel.org>,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Sylwester Nawrocki <s.nawrocki@...sung.com>,
	Mauro Carvalho Chehab <mchehab@....samsung.com>,
	Kukjin Kim <kgene@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Alan Stern <stern@...land.harvard.edu>,
	Tony Prisk <linux@...sktech.co.nz>,
	Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>,
	Tomi Valkeinen <tomi.valkeinen@...com>,
	Arnd Bergmann <arnd@...db.de>,
	Mathias Nyman <mathias.nyman@...ux.intel.com>,
	Paul Bolle <pebolle@...cali.nl>,
	Thomas Pugliese <thomas.pugliese@...il.com>,
	Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
	Masanari Iida <standby24x7@...il.com>,
	David Mosberger <davidm@...uge.net>,
	Peter Griffin <peter.griffin@...aro.org>,
	Gregory CLEMENT <gregory.clement@...e-electrons.com>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Kevin Hao <haokexin@...il.com>,
	Jean Delvare <jdelvare@...e.de>, <linux-doc@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-ide@...r.kernel.org>,
	<linux-media@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>,
	<linux-samsung-soc@...r.kernel.org>, <linux-usb@...r.kernel.org>,
	<linux-fbdev@...r.kernel.org>, Dmitry Torokhov <dtor@...gle.com>,
	Anatol Pomazau <anatol@...gle.com>,
	Jonathan Richardson <jonathar@...adcom.com>,
	Scott Branden <sbranden@...adcom.com>,
	Ray Jui <rjui@...adcom.com>,
	<bcm-kernel-feedback-list@...adcom.com>
Subject: Re: [PATCHv3 1/4] phy: phy-core: Make GENERIC_PHY an invisible option

Hi,

On Fri, May 29, 2015 at 05:04:38PM +0530, Kishon Vijay Abraham I wrote:
> Hi Felipe,
> 
> On Wednesday 27 May 2015 12:09 AM, Felipe Balbi wrote:
> >On Tue, May 26, 2015 at 11:37:17AM -0700, Arun Ramamurthy wrote:
> >>Hi
> >>
> >>On 15-05-26 07:19 AM, Felipe Balbi wrote:
> >>>HI,
> >>>
> >>>On Mon, May 25, 2015 at 02:19:58PM -0700, Arun Ramamurthy wrote:
> >>>>
> >>>>
> >>>>On 15-05-14 05:52 PM, Felipe Balbi wrote:
> >>>>>Hi,
> >>>>>
> >>>>>On Wed, Apr 22, 2015 at 04:04:10PM -0700, Arun Ramamurthy wrote:
> >>>>>>Most of the phy providers use "select" to enable GENERIC_PHY. Since select
> >>>>>>is only recommended when the config is not visible, GENERIC_PHY is changed
> >>>>>>an invisible option. To maintain consistency, all phy providers are changed
> >>>>>>to "select" GENERIC_PHY and all non-phy drivers use "depends on" when the
> >>>>>>phy framework is explicity required. USB_MUSB_OMAP2PLUS has a cyclic
> >>>>>>dependency, so it is left as "select".
> >>>>>>
> >>>>>>Signed-off-by: Arun Ramamurthy <arun.ramamurthy@...adcom.com>
> >>>>>>---
> >>>>>>  drivers/ata/Kconfig                       | 1 -
> >>>>>>  drivers/media/platform/exynos4-is/Kconfig | 2 +-
> >>>>>>  drivers/phy/Kconfig                       | 4 ++--
> >>>>>>  drivers/usb/host/Kconfig                  | 4 ++--
> >>>>>>  drivers/video/fbdev/exynos/Kconfig        | 2 +-
> >>>>>>  5 files changed, 6 insertions(+), 7 deletions(-)
> >>>>>>
> >>>>>>diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> >>>>>>index 5f60155..6d2e881 100644
> >>>>>>--- a/drivers/ata/Kconfig
> >>>>>>+++ b/drivers/ata/Kconfig
> >>>>>>@@ -301,7 +301,6 @@ config SATA_MV
> >>>>>>  	tristate "Marvell SATA support"
> >>>>>>  	depends on PCI || ARCH_DOVE || ARCH_MV78XX0 || \
> >>>>>>  		   ARCH_MVEBU || ARCH_ORION5X || COMPILE_TEST
> >>>>>>-	select GENERIC_PHY
> >>>>>>  	help
> >>>>>>  	  This option enables support for the Marvell Serial ATA family.
> >>>>>>  	  Currently supports 88SX[56]0[48][01] PCI(-X) chips,
> >>>>>>diff --git a/drivers/media/platform/exynos4-is/Kconfig b/drivers/media/platform/exynos4-is/Kconfig
> >>>>>>index b7b2e47..b6f3eaa 100644
> >>>>>>--- a/drivers/media/platform/exynos4-is/Kconfig
> >>>>>>+++ b/drivers/media/platform/exynos4-is/Kconfig
> >>>>>>@@ -31,7 +31,7 @@ config VIDEO_S5P_FIMC
> >>>>>>  config VIDEO_S5P_MIPI_CSIS
> >>>>>>  	tristate "S5P/EXYNOS MIPI-CSI2 receiver (MIPI-CSIS) driver"
> >>>>>>  	depends on REGULATOR
> >>>>>>-	select GENERIC_PHY
> >>>>>>+	depends on GENERIC_PHY
> >>>>>>  	help
> >>>>>>  	  This is a V4L2 driver for Samsung S5P and EXYNOS4 SoC MIPI-CSI2
> >>>>>>  	  receiver (MIPI-CSIS) devices.
> >>>>>>diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> >>>>>>index 2962de2..edecdb1 100644
> >>>>>>--- a/drivers/phy/Kconfig
> >>>>>>+++ b/drivers/phy/Kconfig
> >>>>>>@@ -5,7 +5,7 @@
> >>>>>>  menu "PHY Subsystem"
> >>>>>>
> >>>>>>  config GENERIC_PHY
> >>>>>>-	bool "PHY Core"
> >>>>>>+	bool
> >>>>>>  	help
> >>>>>>  	  Generic PHY support.
> >>>>>>
> >>>>>>@@ -72,7 +72,7 @@ config PHY_MIPHY365X
> >>>>>>  config PHY_RCAR_GEN2
> >>>>>>  	tristate "Renesas R-Car generation 2 USB PHY driver"
> >>>>>>  	depends on ARCH_SHMOBILE
> >>>>>>-	depends on GENERIC_PHY
> >>>>>>+	select GENERIC_PHY
> >>>>>
> >>>>>so some you changed from depends to select...
> >>>>>
> >>>>>>  	help
> >>>>>>  	  Support for USB PHY found on Renesas R-Car generation 2 SoCs.
> >>>>>>
> >>>>>>diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> >>>>>>index 5ad60e4..e2197e2 100644
> >>>>>>--- a/drivers/usb/host/Kconfig
> >>>>>>+++ b/drivers/usb/host/Kconfig
> >>>>>>@@ -182,7 +182,7 @@ config USB_EHCI_HCD_SPEAR
> >>>>>>  config USB_EHCI_HCD_STI
> >>>>>>  	tristate "Support for ST STiHxxx on-chip EHCI USB controller"
> >>>>>>  	depends on ARCH_STI && OF
> >>>>>>-	select GENERIC_PHY
> >>>>>>+	depends on GENERIC_PHY
> >>>>>
> >>>>>while others you changed from select to depends.
> >>>>>
> >>>>>NAK.
> >>>>>
> >>>>Felipe, I dont understand your concern, could you please explain it more
> >>>>detail?  The logic behind the changes is that in cases where there was an
> >>>>explicit dependency, I changed it to "depends on" and in other cases I
> >>>>changed it to "selects". Thanks
> >>>
> >>>Since GENERIC_PHY is visible from Kconfig, it would be much nicer to
> >>>avoid select altogether.
> >>>
> >>Felipe, after discussion with the maintainers, I have made GENERIC_PHY an
> >>invisible option as part of this change. Thanks
> >
> >Then, if the option is invisible, how can you "depend" on it ? It can
> >never be selected by poking around in Kconfig. IMO, it's
> >counterintuitive that you need to enable a PHY driver before you can see
> >your EHCI/OHCI/whatever controller listed in Kconfig.
> 
> If the controller requires PHY for it to be functional, it is okay to make
> the controller depend on PHY IMHO. We want to try and minimize the usage of
> 'select' wherever possible or else 'select' is the most intuitive way. The
> other option is just to leave the 'depends on' and let the user select PHY.

How can you 'depend' on something that the user can't select by
navigating through Kconfig ?

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)