Re: [PATCH v9 1/8] drivers: phy: add generic PHY framework
Hi, On Wednesday 17 July 2013 10:55 PM, Greg KH wrote: On Wed, Jul 17, 2013 at 03:02:59PM +0530, Kishon Vijay Abraham I wrote: Hi, On Wednesday 17 July 2013 11:59 AM, Greg KH wrote: On Wed, Jun 26, 2013 at 05:17:29PM +0530, Kishon Vijay Abraham I wrote: +menuconfig GENERIC_PHY + tristate PHY Subsystem + help +Generic PHY support. + +This framework is designed to provide a generic interface for PHY +devices present in the kernel. This layer will have the generic +API by which phy drivers can create PHY using the phy framework and +phy users can obtain reference to the PHY. Shouldn't this be something that other drivers select? How will anyone know if they need this or not? All the PHY drivers should go here. So only if *GENERIC_PHY* is enabled those PHY drivers can be selected like in [1]. The PHY consumer driver should ideally use *depends on* in their Kconfig. [1] - http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html No, switch it around the other way. How would I even _know_ that I need to enable the generic PHY subsystem in the first place? How can I determine that I need this for my hardware? You need to give hints to someone who doesn't even know what a PHY is, otherwise they will always disable it and move on. --- /dev/null +++ b/drivers/phy/phy-core.c @@ -0,0 +1,544 @@ +/* + * phy-core.c -- Generic Phy framework. + * + * Copyright (C) 2013 Texas Instruments + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. You really mean any later version (I have to ask)? That was copied from somewhere :-s So, is that what you really mean to have for this code? indeed. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. Are these two paragraphs needed? This isn't a program, and they got a copy of the GPL already with the kernel. hmm.. I can remove that. +static struct class *phy_class; Why do you need a class? Wanted to group all the PHY drivers to be used by different subsystems (SATA/USB/PCIE/HDMI/VIDEO) into a single entity. There were some comments in my initial version [3] on using a bus_type instead of class but then it was decided to go with class itself. [3] - http://lkml.indiana.edu/hypermail/linux/kernel/1302.2/01389.html Ok, but what does the class usage get you? hmm.. actually I use class only to iterate through the list of devices in *phy* class which could very well be implemented using list. Just that I wont have a /sys/class/phy/ entry to find the list of phys added in the system. I dont think I want to add any other stuff to expose to the user space at this point of time. When modifying/adding new sysfs stuff, you need a Documentation/ABI/ entry as well. I'm not actually adding any new sysfs entry other than what a *class_create* must have created. Do I need to add one for that? If you are not creating anything in sysfs at all, why use the driver model? (hint, I think you need to do something here to justify it...) Well.. it helps me to use pm_runtime to enable clocks utilizing the parent-child relationship. Thanks Kishon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/8] drivers: phy: add generic PHY framework
On Thu, Jul 18, 2013 at 11:33:17AM +0530, Kishon Vijay Abraham I wrote: Wanted to group all the PHY drivers to be used by different subsystems (SATA/USB/PCIE/HDMI/VIDEO) into a single entity. There were some comments in my initial version [3] on using a bus_type instead of class but then it was decided to go with class itself. [3] - http://lkml.indiana.edu/hypermail/linux/kernel/1302.2/01389.html Ok, but what does the class usage get you? hmm.. actually I use class only to iterate through the list of devices in *phy* class which could very well be implemented using list. Just that I wont have a /sys/class/phy/ entry to find the list of phys added in the system. I dont think I want to add any other stuff to expose to the user space at this point of time. When modifying/adding new sysfs stuff, you need a Documentation/ABI/ entry as well. I'm not actually adding any new sysfs entry other than what a *class_create* must have created. Do I need to add one for that? If you are not creating anything in sysfs at all, why use the driver model? (hint, I think you need to do something here to justify it...) Well.. it helps me to use pm_runtime to enable clocks utilizing the parent-child relationship. Ok, that's a good reason for this, nevermind then. Care to send the latest patches you have in emails so I can review them? thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/8] drivers: phy: add generic PHY framework
On Thursday 18 July 2013 11:54 AM, Greg KH wrote: On Thu, Jul 18, 2013 at 11:33:17AM +0530, Kishon Vijay Abraham I wrote: Wanted to group all the PHY drivers to be used by different subsystems (SATA/USB/PCIE/HDMI/VIDEO) into a single entity. There were some comments in my initial version [3] on using a bus_type instead of class but then it was decided to go with class itself. [3] - http://lkml.indiana.edu/hypermail/linux/kernel/1302.2/01389.html Ok, but what does the class usage get you? hmm.. actually I use class only to iterate through the list of devices in *phy* class which could very well be implemented using list. Just that I wont have a /sys/class/phy/ entry to find the list of phys added in the system. I dont think I want to add any other stuff to expose to the user space at this point of time. When modifying/adding new sysfs stuff, you need a Documentation/ABI/ entry as well. I'm not actually adding any new sysfs entry other than what a *class_create* must have created. Do I need to add one for that? If you are not creating anything in sysfs at all, why use the driver model? (hint, I think you need to do something here to justify it...) Well.. it helps me to use pm_runtime to enable clocks utilizing the parent-child relationship. Ok, that's a good reason for this, nevermind then. Care to send the latest patches you have in emails so I can review them? Sure. Thanks Kishon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/8] drivers: phy: add generic PHY framework
On Wed, Jun 26, 2013 at 05:17:29PM +0530, Kishon Vijay Abraham I wrote: +menuconfig GENERIC_PHY + tristate PHY Subsystem + help + Generic PHY support. + + This framework is designed to provide a generic interface for PHY + devices present in the kernel. This layer will have the generic + API by which phy drivers can create PHY using the phy framework and + phy users can obtain reference to the PHY. Shouldn't this be something that other drivers select? How will anyone know if they need this or not? --- /dev/null +++ b/drivers/phy/phy-core.c @@ -0,0 +1,544 @@ +/* + * phy-core.c -- Generic Phy framework. + * + * Copyright (C) 2013 Texas Instruments + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. You really mean any later version (I have to ask)? + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. Are these two paragraphs needed? This isn't a program, and they got a copy of the GPL already with the kernel. +static struct class *phy_class; Why do you need a class? When modifying/adding new sysfs stuff, you need a Documentation/ABI/ entry as well. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/8] drivers: phy: add generic PHY framework
Hi, On Wednesday 17 July 2013 11:59 AM, Greg KH wrote: On Wed, Jun 26, 2013 at 05:17:29PM +0530, Kishon Vijay Abraham I wrote: +menuconfig GENERIC_PHY +tristate PHY Subsystem +help + Generic PHY support. + + This framework is designed to provide a generic interface for PHY + devices present in the kernel. This layer will have the generic + API by which phy drivers can create PHY using the phy framework and + phy users can obtain reference to the PHY. Shouldn't this be something that other drivers select? How will anyone know if they need this or not? All the PHY drivers should go here. So only if *GENERIC_PHY* is enabled those PHY drivers can be selected like in [1]. The PHY consumer driver should ideally use *depends on* in their Kconfig. [1] - http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html --- /dev/null +++ b/drivers/phy/phy-core.c @@ -0,0 +1,544 @@ +/* + * phy-core.c -- Generic Phy framework. + * + * Copyright (C) 2013 Texas Instruments + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. You really mean any later version (I have to ask)? That was copied from somewhere :-s + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. Are these two paragraphs needed? This isn't a program, and they got a copy of the GPL already with the kernel. hmm.. I can remove that. +static struct class *phy_class; Why do you need a class? Wanted to group all the PHY drivers to be used by different subsystems (SATA/USB/PCIE/HDMI/VIDEO) into a single entity. There were some comments in my initial version [3] on using a bus_type instead of class but then it was decided to go with class itself. [3] - http://lkml.indiana.edu/hypermail/linux/kernel/1302.2/01389.html When modifying/adding new sysfs stuff, you need a Documentation/ABI/ entry as well. I'm not actually adding any new sysfs entry other than what a *class_create* must have created. Do I need to add one for that? Thanks Kishon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/8] drivers: phy: add generic PHY framework
On Wed, Jul 17, 2013 at 03:02:59PM +0530, Kishon Vijay Abraham I wrote: Hi, On Wednesday 17 July 2013 11:59 AM, Greg KH wrote: On Wed, Jun 26, 2013 at 05:17:29PM +0530, Kishon Vijay Abraham I wrote: +menuconfig GENERIC_PHY + tristate PHY Subsystem + help +Generic PHY support. + +This framework is designed to provide a generic interface for PHY +devices present in the kernel. This layer will have the generic +API by which phy drivers can create PHY using the phy framework and +phy users can obtain reference to the PHY. Shouldn't this be something that other drivers select? How will anyone know if they need this or not? All the PHY drivers should go here. So only if *GENERIC_PHY* is enabled those PHY drivers can be selected like in [1]. The PHY consumer driver should ideally use *depends on* in their Kconfig. [1] - http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html No, switch it around the other way. How would I even _know_ that I need to enable the generic PHY subsystem in the first place? How can I determine that I need this for my hardware? You need to give hints to someone who doesn't even know what a PHY is, otherwise they will always disable it and move on. --- /dev/null +++ b/drivers/phy/phy-core.c @@ -0,0 +1,544 @@ +/* + * phy-core.c -- Generic Phy framework. + * + * Copyright (C) 2013 Texas Instruments + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. You really mean any later version (I have to ask)? That was copied from somewhere :-s So, is that what you really mean to have for this code? + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. Are these two paragraphs needed? This isn't a program, and they got a copy of the GPL already with the kernel. hmm.. I can remove that. +static struct class *phy_class; Why do you need a class? Wanted to group all the PHY drivers to be used by different subsystems (SATA/USB/PCIE/HDMI/VIDEO) into a single entity. There were some comments in my initial version [3] on using a bus_type instead of class but then it was decided to go with class itself. [3] - http://lkml.indiana.edu/hypermail/linux/kernel/1302.2/01389.html Ok, but what does the class usage get you? When modifying/adding new sysfs stuff, you need a Documentation/ABI/ entry as well. I'm not actually adding any new sysfs entry other than what a *class_create* must have created. Do I need to add one for that? If you are not creating anything in sysfs at all, why use the driver model? (hint, I think you need to do something here to justify it...) thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/8] drivers: phy: add generic PHY framework
On Wed, 26 Jun 2013 17:17:29 +0530, Kishon Vijay Abraham Iwrote: The PHY framework provides a set of APIs for the PHY drivers to create/destroy a PHY and APIs for the PHY users to obtain a reference to the PHY with or without using phandle. For dt-boot, the PHY drivers should also register *PHY provider* with the framework. PHY drivers should create the PHY by passing id and ops like init, exit, power_on and power_off. This framework is also pm runtime enabled. The documentation for the generic PHY framework is added in Documentation/phy.txt and the documentation for dt binding can be found at Documentation/devicetree/bindings/phy/phy-bindings.txt Signed-off-by: Kishon Vijay Abraham I kis...@ti.com Tested-by: Sylwester Nawrocki s.nawro...@samsung.com Tested-by: Jingoo Han jg1@samsung.com It looks great to me! I tested this General PHY framework with Exynos5250 eDP. It works properly. I will share the patch 'Generic PHY driver for the Exynos SoC DP PHY', soon. Thanks. Best regards, Jingoo Han --- .../devicetree/bindings/phy/phy-bindings.txt | 66 +++ Documentation/phy.txt | 129 + MAINTAINERS|7 + drivers/Kconfig|2 + drivers/Makefile |2 + drivers/phy/Kconfig| 13 + drivers/phy/Makefile |5 + drivers/phy/phy-core.c | 544 include/linux/phy/phy.h| 344 + 9 files changed, 1112 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt create mode 100644 Documentation/phy.txt create mode 100644 drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create mode 100644 drivers/phy/phy-core.c create mode 100644 include/linux/phy/phy.h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/8] drivers: phy: add generic PHY framework
On Friday 28 June 2013 08:57 AM, Jingoo Han wrote: On Wed, 26 Jun 2013 17:17:29 +0530, Kishon Vijay Abraham Iwrote: The PHY framework provides a set of APIs for the PHY drivers to create/destroy a PHY and APIs for the PHY users to obtain a reference to the PHY with or without using phandle. For dt-boot, the PHY drivers should also register *PHY provider* with the framework. PHY drivers should create the PHY by passing id and ops like init, exit, power_on and power_off. This framework is also pm runtime enabled. The documentation for the generic PHY framework is added in Documentation/phy.txt and the documentation for dt binding can be found at Documentation/devicetree/bindings/phy/phy-bindings.txt Signed-off-by: Kishon Vijay Abraham I kis...@ti.com Tested-by: Sylwester Nawrocki s.nawro...@samsung.com Tested-by: Jingoo Han jg1@samsung.com It looks great to me! I tested this General PHY framework with Exynos5250 eDP. It works properly. Thanks for testing this :-) I will share the patch 'Generic PHY driver for the Exynos SoC DP PHY', soon. Thanks. Cool. Thanks Kishon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 1/8] drivers: phy: add generic PHY framework
The PHY framework provides a set of APIs for the PHY drivers to
create/destroy a PHY and APIs for the PHY users to obtain a reference to the
PHY with or without using phandle. For dt-boot, the PHY drivers should
also register *PHY provider* with the framework.
PHY drivers should create the PHY by passing id and ops like init, exit,
power_on and power_off. This framework is also pm runtime enabled.
The documentation for the generic PHY framework is added in
Documentation/phy.txt and the documentation for dt binding can be found at
Documentation/devicetree/bindings/phy/phy-bindings.txt
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Tested-by: Sylwester Nawrocki s.nawro...@samsung.com
---
.../devicetree/bindings/phy/phy-bindings.txt | 66 +++
Documentation/phy.txt | 129 +
MAINTAINERS|7 +
drivers/Kconfig|2 +
drivers/Makefile |2 +
drivers/phy/Kconfig| 13 +
drivers/phy/Makefile |5 +
drivers/phy/phy-core.c | 544
include/linux/phy/phy.h| 344 +
9 files changed, 1112 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
create mode 100644 Documentation/phy.txt
create mode 100644 drivers/phy/Kconfig
create mode 100644 drivers/phy/Makefile
create mode 100644 drivers/phy/phy-core.c
create mode 100644 include/linux/phy/phy.h
diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt
b/Documentation/devicetree/bindings/phy/phy-bindings.txt
new file mode 100644
index 000..8ae844f
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
@@ -0,0 +1,66 @@
+This document explains only the device tree data binding. For general
+information about PHY subsystem refer to Documentation/phy.txt
+
+PHY device node
+===
+
+Required Properties:
+#phy-cells:Number of cells in a PHY specifier; The meaning of all those
+ cells is defined by the binding for the phy node. The PHY
+ provider can use the values in cells to find the appropriate
+ PHY.
+
+For example:
+
+phys: phy {
+compatible = xxx;
+reg = ...;
+.
+.
+#phy-cells = 1;
+.
+.
+};
+
+That node describes an IP block (PHY provider) that implements 2 different
PHYs.
+In order to differentiate between these 2 PHYs, an additonal specifier should
be
+given while trying to get a reference to it.
+
+PHY user node
+=
+
+Required Properties:
+phys : the phandle for the PHY device (used by the PHY subsystem)
+phy-names : the names of the PHY corresponding to the PHYs present in the
+ *phys* phandle
+
+Example 1:
+usb1: usb_otg_ss@xxx {
+compatible = xxx;
+reg = xxx;
+.
+.
+phys = usb2_phy, usb3_phy;
+phy-names = usb2phy, usb3phy;
+.
+.
+};
+
+This node represents a controller that uses two PHYs, one for usb2 and one for
+usb3.
+
+Example 2:
+usb2: usb_otg_ss@xxx {
+compatible = xxx;
+reg = xxx;
+.
+.
+phys = phys 1;
+phy-names = usbphy;
+.
+.
+};
+
+This node represents a controller that uses one of the PHYs of the PHY provider
+device defined previously. Note that the phy handle has an additional specifier
+1 to differentiate between the two PHYs.
diff --git a/Documentation/phy.txt b/Documentation/phy.txt
new file mode 100644
index 000..05f8fda
--- /dev/null
+++ b/Documentation/phy.txt
@@ -0,0 +1,129 @@
+ PHY SUBSYSTEM
+ Kishon Vijay Abraham I kis...@ti.com
+
+This document explains the Generic PHY Framework along with the APIs provided,
+and how-to-use.
+
+1. Introduction
+
+*PHY* is the abbreviation for physical layer. It is used to connect a device
+to the physical medium e.g., the USB controller has a PHY to provide functions
+such as serialization, de-serialization, encoding, decoding and is responsible
+for obtaining the required data transmission rate. Note that some USB
+controllers have PHY functionality embedded into it and others use an external
+PHY. Other peripherals that use PHY include Wireless LAN, Ethernet,
+SATA etc.
+
+The intention of creating this framework is to bring the PHY drivers spread
+all over the Linux kernel to drivers/phy to increase code re-use and for
+better code maintainability.
+
+This framework will be of use only to devices that use external PHY (PHY
+functionality is not embedded within the controller).
+
+2. Registering/Unregistering the PHY provider
+
+PHY provider refers to an entity that implements one or more PHY instances.
+For the simple case where the PHY provider implements only a single instance of
+the PHY, the framework provides its own implementation of of_xlate in
+of_phy_simple_xlate. If the PHY provider
Re: [PATCH v9 1/8] drivers: phy: add generic PHY framework
On Wed, Jun 26, 2013 at 05:17:29PM +0530, Kishon Vijay Abraham I wrote: The PHY framework provides a set of APIs for the PHY drivers to create/destroy a PHY and APIs for the PHY users to obtain a reference to the PHY with or without using phandle. For dt-boot, the PHY drivers should also register *PHY provider* with the framework. PHY drivers should create the PHY by passing id and ops like init, exit, power_on and power_off. This framework is also pm runtime enabled. The documentation for the generic PHY framework is added in Documentation/phy.txt and the documentation for dt binding can be found at Documentation/devicetree/bindings/phy/phy-bindings.txt Signed-off-by: Kishon Vijay Abraham I kis...@ti.com Tested-by: Sylwester Nawrocki s.nawro...@samsung.com looks great to my eyes Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
