Re: [PATCH v3 1/4] pinctrl: add samsung pinctrl and gpiolib driver

2012-09-06 Thread Stephen Warren
On 09/05/2012 12:20 AM, Thomas Abraham wrote:
 Add a new device tree enabled pinctrl and gpiolib driver for Samsung
 SoC's. This driver provides a common and extensible framework for all
 Samsung SoC's to interface with the pinctrl and gpiolib subsystems. This
 driver supports only device tree based instantiation and hence can be
 used only on those Samsung platforms that have device tree enabled.
 
 This driver is split into two parts: the pinctrl interface and the gpiolib
 interface. The pinctrl interface registers pinctrl devices with the pinctrl
 subsystem and gpiolib interface registers gpio chips with the gpiolib
 subsystem. The information about the pins, pin groups, pin functions and
 gpio chips, which are SoC specific, are parsed from device tree node.
 
 Cc: Linus Walleij linus.wall...@linaro.org
 Cc: Kukjin Kim kgene@samsung.com
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org

Acked-by: Stephen Warren swar...@nvidia.com

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v3 1/4] pinctrl: add samsung pinctrl and gpiolib driver

2012-09-06 Thread Kukjin Kim
Stephen Warren wrote:
 
 On 09/05/2012 12:20 AM, Thomas Abraham wrote:
  Add a new device tree enabled pinctrl and gpiolib driver for Samsung
  SoC's. This driver provides a common and extensible framework for all
  Samsung SoC's to interface with the pinctrl and gpiolib subsystems. This
  driver supports only device tree based instantiation and hence can be
  used only on those Samsung platforms that have device tree enabled.
 
  This driver is split into two parts: the pinctrl interface and the
 gpiolib
  interface. The pinctrl interface registers pinctrl devices with the
 pinctrl
  subsystem and gpiolib interface registers gpio chips with the gpiolib
  subsystem. The information about the pins, pin groups, pin functions and
  gpio chips, which are SoC specific, are parsed from device tree node.
 
  Cc: Linus Walleij linus.wall...@linaro.org
  Cc: Kukjin Kim kgene@samsung.com
  Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
 
 Acked-by: Stephen Warren swar...@nvidia.com

Applied this whole series, thanks.

Best regards,
Kgene.
--
Kukjin Kim kgene@samsung.com, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 1/4] pinctrl: add samsung pinctrl and gpiolib driver

2012-09-05 Thread Tomasz Figa
Hi Thomas,

Thomas Abraham wrote:
 Add a new device tree enabled pinctrl and gpiolib driver for Samsung
 SoC's. This driver provides a common and extensible framework for all
 Samsung SoC's to interface with the pinctrl and gpiolib subsystems. This
 driver supports only device tree based instantiation and hence can be
 used only on those Samsung platforms that have device tree enabled.
 
 This driver is split into two parts: the pinctrl interface and the gpiolib
 interface. The pinctrl interface registers pinctrl devices with the
 pinctrl subsystem and gpiolib interface registers gpio chips with the
 gpiolib subsystem. The information about the pins, pin groups, pin
 functions and gpio chips, which are SoC specific, are parsed from device
 tree node.
 
 Cc: Linus Walleij linus.wall...@linaro.org
 Cc: Kukjin Kim kgene@samsung.com
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org

Does the driver provide any kind of compatibility with current gpiolib 
users?

Let me show an example of what I mean. We have a fixed voltage regulator 
defined in device tree of an imaginary board

vemmc_reg: voltage-regulator@0 {
compatible = regulator-fixed;
regulator-name = VMEM_VDD_2.8V;
regulator-min-microvolt = 280;
regulator-max-microvolt = 280;
gpio = gpk0 2 1 0 0;
enable-active-high;
};

The gpio pin used to control status of the regulator is defined using the 
gpio property and regulator-fixed driver uses of_get_named_gpio to get the 
pin number from device tree.

Is this kind of setup also valid when using your pinctrl driver?

Best regards,
-- 
Tomasz Figa
Samsung Poland RD Center


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 1/4] pinctrl: add samsung pinctrl and gpiolib driver

2012-09-05 Thread Thomas Abraham
On 5 September 2012 19:20, Tomasz Figa t.f...@samsung.com wrote:
 Hi Thomas,

 Thomas Abraham wrote:
 Add a new device tree enabled pinctrl and gpiolib driver for Samsung
 SoC's. This driver provides a common and extensible framework for all
 Samsung SoC's to interface with the pinctrl and gpiolib subsystems. This
 driver supports only device tree based instantiation and hence can be
 used only on those Samsung platforms that have device tree enabled.

 This driver is split into two parts: the pinctrl interface and the gpiolib
 interface. The pinctrl interface registers pinctrl devices with the
 pinctrl subsystem and gpiolib interface registers gpio chips with the
 gpiolib subsystem. The information about the pins, pin groups, pin
 functions and gpio chips, which are SoC specific, are parsed from device
 tree node.

 Cc: Linus Walleij linus.wall...@linaro.org
 Cc: Kukjin Kim kgene@samsung.com
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org

 Does the driver provide any kind of compatibility with current gpiolib
 users?

 Let me show an example of what I mean. We have a fixed voltage regulator
 defined in device tree of an imaginary board

 vemmc_reg: voltage-regulator@0 {
 compatible = regulator-fixed;
 regulator-name = VMEM_VDD_2.8V;
 regulator-min-microvolt = 280;
 regulator-max-microvolt = 280;
 gpio = gpk0 2 1 0 0;
 enable-active-high;
 };

 The gpio pin used to control status of the regulator is defined using the
 gpio property and regulator-fixed driver uses of_get_named_gpio to get the
 pin number from device tree.

 Is this kind of setup also valid when using your pinctrl driver?

Hi Tomasz,

It is possible to get the pin number using of_get_named_gpio()
function with the gpiolib support included in the Samsung pinctrl
driver. The following are the steps on how this can be done. Please
note that, the old Samsung gpio binding will change when we switch the
pinctrl driver. But that was inevitable.

1. The gpio binding will now follow the standard gpio binding.
Which means, it will be something like

   uart@1380 {
 the usual bindings here
 gpios = pinctrl_0 10 0;
   };

   Note that,

   A. First cell: the pin-controller instance is used as the phandle
(i.e pinctrl_0).
   B. Second cell: the pin number __local__ to the pin controller instance.
   C. Third cell: Flags associated with the gpio pin (as per standard binding).

   The pin number specified in the second cell is local to the
pin-controller instance.

   For example, the Exynos4210 GPJ1[1] pin, which belongs to pinctrl instance 1,
   will have pin number (pin number local to pinctrl instance 1) as 10.

   That is because, the number of pins in GPJ0 is 8. So GPJ1[1] =
nr_pins(GPJ0) + 2
   = 8 + 2 = 10.

   Yes, I understand that this is hard to derive the pin number like
this and write
   in the dts(i) files, but while writing the pinctrl driver, there
were two thoughts.

   A. The pin numbers for all the pins included in a pinctrl instance
will be documented
in the device tree binding documentation. So it can be easily looked up.

   B. How many instances of listing gpio number like this will ever be
needed for a
   board dts file. Not much, maybe atmost 10.

   Considering the two points above, it seemed okay to derive the pin
numbers for
   the gpio as listed in the above example.


2. The client driver can now lookup the pin number using

gpio = of_get_gpio(dev-of_node);

which returns the pin number in the linux gpio space.

So, it is possible to lookup the pin number using Samsung pinctrl driver.

But, I have missed adding the #gpio-cells property in the three
instances of the pinctrl driver. If you are trying this, please add

  #gpio-cells = 2;

line in all the three instances of the pinctrl controller nodes. I
will send this change as a separate patch.

Thanks,
Thomas.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 1/4] pinctrl: add samsung pinctrl and gpiolib driver

2012-09-04 Thread Thomas Abraham
On 3 September 2012 16:44, Linus Walleij linus.wall...@linaro.org wrote:
 On Thu, Aug 23, 2012 at 1:15 PM, Thomas Abraham
 thomas.abra...@linaro.org wrote:

 Add a new device tree enabled pinctrl and gpiolib driver for Samsung
 SoC's. This driver provides a common and extensible framework for all
 Samsung SoC's to interface with the pinctrl and gpiolib subsystems. This
 driver supports only device tree based instantiation and hence can be
 used only on those Samsung platforms that have device tree enabled.

 This driver is split into two parts: the pinctrl interface and the gpiolib
 interface. The pinctrl interface registers pinctrl devices with the pinctrl
 subsystem and gpiolib interface registers gpio chips with the gpiolib
 subsystem. The information about the pins, pin groups, pin functions and
 gpio chips, which are SoC specific, are parsed from device tree node.

 Cc: Linus Walleij linus.wall...@linaro.org
 Cc: Kukjin Kim kgene@samsung.com
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org

 Looks good to me, I saw Stephen had some minor comments and
 I expect that you probably fix them before applying to the Samsung
 tree so:
 Reviewed-by: Linus Walleij linus.wall...@linaro.org

 Feel free to push this through ARM SoC, I guess that's the plan?

Hi Linus,

Thanks for reviewing the Samsung pinctrl driver patches. I will do the
changes that Stephen has listed and resubmit. I will request Samsung
maintainer to consider the support for pinctrl driver for 3.7.

Thanks,
Thomas.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v3 1/4] pinctrl: add samsung pinctrl and gpiolib driver

2012-09-04 Thread Kukjin Kim
Thomas Abraham wrote:
 
 On 3 September 2012 16:44, Linus Walleij linus.wall...@linaro.org wrote:
  On Thu, Aug 23, 2012 at 1:15 PM, Thomas Abraham
  thomas.abra...@linaro.org wrote:
 
  Add a new device tree enabled pinctrl and gpiolib driver for Samsung
  SoC's. This driver provides a common and extensible framework for all
  Samsung SoC's to interface with the pinctrl and gpiolib subsystems.
 This
  driver supports only device tree based instantiation and hence can be
  used only on those Samsung platforms that have device tree enabled.
 
  This driver is split into two parts: the pinctrl interface and the
 gpiolib
  interface. The pinctrl interface registers pinctrl devices with the
 pinctrl
  subsystem and gpiolib interface registers gpio chips with the gpiolib
  subsystem. The information about the pins, pin groups, pin functions
 and
  gpio chips, which are SoC specific, are parsed from device tree node.
 
  Cc: Linus Walleij linus.wall...@linaro.org
  Cc: Kukjin Kim kgene@samsung.com
  Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
 
  Looks good to me, I saw Stephen had some minor comments and
  I expect that you probably fix them before applying to the Samsung
  tree so:
  Reviewed-by: Linus Walleij linus.wall...@linaro.org
 
  Feel free to push this through ARM SoC, I guess that's the plan?
 
 Hi Linus,
 
 Thanks for reviewing the Samsung pinctrl driver patches. I will do the
 changes that Stephen has listed and resubmit. I will request Samsung
 maintainer to consider the support for pinctrl driver for 3.7.
 
Looks OK, I will apply updated patches Thomas said into Samsung tree.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim kgene@samsung.com, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 1/4] pinctrl: add samsung pinctrl and gpiolib driver

2012-09-03 Thread Linus Walleij
On Thu, Aug 23, 2012 at 1:15 PM, Thomas Abraham
thomas.abra...@linaro.org wrote:

 Add a new device tree enabled pinctrl and gpiolib driver for Samsung
 SoC's. This driver provides a common and extensible framework for all
 Samsung SoC's to interface with the pinctrl and gpiolib subsystems. This
 driver supports only device tree based instantiation and hence can be
 used only on those Samsung platforms that have device tree enabled.

 This driver is split into two parts: the pinctrl interface and the gpiolib
 interface. The pinctrl interface registers pinctrl devices with the pinctrl
 subsystem and gpiolib interface registers gpio chips with the gpiolib
 subsystem. The information about the pins, pin groups, pin functions and
 gpio chips, which are SoC specific, are parsed from device tree node.

 Cc: Linus Walleij linus.wall...@linaro.org
 Cc: Kukjin Kim kgene@samsung.com
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org

Looks good to me, I saw Stephen had some minor comments and
I expect that you probably fix them before applying to the Samsung
tree so:
Reviewed-by: Linus Walleij linus.wall...@linaro.org

Feel free to push this through ARM SoC, I guess that's the plan?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 1/4] pinctrl: add samsung pinctrl and gpiolib driver

2012-08-23 Thread Stephen Warren
On 08/23/2012 05:15 AM, Thomas Abraham wrote:
 Add a new device tree enabled pinctrl and gpiolib driver for Samsung
 SoC's. This driver provides a common and extensible framework for all
 Samsung SoC's to interface with the pinctrl and gpiolib subsystems. This
 driver supports only device tree based instantiation and hence can be
 used only on those Samsung platforms that have device tree enabled.
 
 This driver is split into two parts: the pinctrl interface and the gpiolib
 interface. The pinctrl interface registers pinctrl devices with the pinctrl
 subsystem and gpiolib interface registers gpio chips with the gpiolib
 subsystem. The information about the pins, pin groups, pin functions and
 gpio chips, which are SoC specific, are parsed from device tree node.

 diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt 
 b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt

BTW, this is a very nicely written and complete/precise binding
document. Well done.

 +Samsung GPIO and Pin Mux/Config controller
 +
 +Samsung's ARM based SoC's integrates a GPIO and Pin mux/config hardware
 +controller. It controls the input/output settings on the available pads/pins
 +and also provides ability to multiplex and configure the output of various
 +on-chip controllers onto these pads.
 +
 +Required Properties:
 +- compatible: should be one of the following.
 +  - samsung,pinctrl-exynos4210: for Exynos4210 compatible pin-controller.
 +  - samsung,pinctrl-exynos5250: for Exynos5250 compatible pin-controller.
 +
 +- reg: Base address of the pin controller hardware module and length of
 +  the address space it occupies.
 +
 +- interrupts: interrupt specifier for the controller. The format and value of
 +  the interrupt specifier depends on the interrupt parent for the controller.
 +
 +- Pin mux/config groups as child nodes: The pin mux (selecting pin function

Direct child nodes of the pin-controller, not a second level?

While that's quite legal, it means that if you need a particular client
module to use 4 pins, 2 of which need one samsung,pin-function value and
2 of which need a different pin-function value, then the client device's
pinctrl-0 property has to have two entries.

i.e. a completely hypothetical example roughly based on yours below:

pinctrl_1: pinctrl@1100 {
uart0_rxd: uart0-rxd {
samsung,pins = gpa0-0;
samsung,pin-function = 2;
samsung,pin-pud = 0;
samsung,pin-drv = 0;
};

uart0_txd: uart0-txd {
samsung,pins = gpa0-1;
samsung,pin-function = 1;
samsung,pin-pud = 0;
samsung,pin-drv = 0;
};
};

uart@1380 {
pinctrl-names = default;
pinctrl-0 = uart0_rxd uart0_txd;
};

rather than:

pinctrl_1: pinctrl@1100 {
uart0_opt1: uart0-opt1 {
uart0_rxd: uart0-rxd {
samsung,pins = gpa0-0;
samsung,pin-function = 2;
samsung,pin-pud = 0;
samsung,pin-drv = 0;
};

uart0_txd: uart0-txd {
samsung,pins = gpa0-1;
samsung,pin-function = 1;
samsung,pin-pud = 0;
samsung,pin-drv = 0;
};
};
};

uart@1380 {
pinctrl-names = default;
pinctrl-0 = uart0_opt1;
};

The latter layout simplifies writing the client nodes, since all the
related settings can be grouped together by whoever writes the pinctrl
node, rather than every client author having to work out all the entries
to include in the list.

That all said, the way you've defined the binding is perfectly
legitimate, and I don't have any kind of issue with it; it's just
something you might want to consider.

Irrespective of whether you choose to keep the binding as-is, or change
it, please consider it:

Acked-by: Stephen Warren swar...@wwwdotorg.org

 +  The values specified by these config properties should be dervied from the

s/dervied/derived/

 +External GPIO and Wakeup Interrupts:
 +
 +The controller supports two types of external interrupts over gpio. The first
 +is the external gpio interrupt and second is the external wakeup interrupts.
 +The difference between the two is that the external wakeup interrupts can be
 +used as system wakeup events.
 +
 +A. External GPIO Interrupts: For supporting external gpio interrupts, the
 +   properties should be specified in the pin-controller device node.

s/the properties/the following properties/ ?

 +Aliases:
 +
 +All the pin controller nodes 

Re: [PATCH v3 1/4] pinctrl: add samsung pinctrl and gpiolib driver

2012-08-23 Thread Thomas Abraham
On 24 August 2012 04:42, Stephen Warren swar...@wwwdotorg.org wrote:
 On 08/23/2012 05:15 AM, Thomas Abraham wrote:
 Add a new device tree enabled pinctrl and gpiolib driver for Samsung
 SoC's. This driver provides a common and extensible framework for all
 Samsung SoC's to interface with the pinctrl and gpiolib subsystems. This
 driver supports only device tree based instantiation and hence can be
 used only on those Samsung platforms that have device tree enabled.

 This driver is split into two parts: the pinctrl interface and the gpiolib
 interface. The pinctrl interface registers pinctrl devices with the pinctrl
 subsystem and gpiolib interface registers gpio chips with the gpiolib
 subsystem. The information about the pins, pin groups, pin functions and
 gpio chips, which are SoC specific, are parsed from device tree node.

 diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt 
 b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt

 BTW, this is a very nicely written and complete/precise binding
 document. Well done.

Thank you!


 +Samsung GPIO and Pin Mux/Config controller
 +
 +Samsung's ARM based SoC's integrates a GPIO and Pin mux/config hardware
 +controller. It controls the input/output settings on the available pads/pins
 +and also provides ability to multiplex and configure the output of various
 +on-chip controllers onto these pads.
 +
 +Required Properties:
 +- compatible: should be one of the following.
 +  - samsung,pinctrl-exynos4210: for Exynos4210 compatible pin-controller.
 +  - samsung,pinctrl-exynos5250: for Exynos5250 compatible pin-controller.
 +
 +- reg: Base address of the pin controller hardware module and length of
 +  the address space it occupies.
 +
 +- interrupts: interrupt specifier for the controller. The format and value 
 of
 +  the interrupt specifier depends on the interrupt parent for the 
 controller.
 +
 +- Pin mux/config groups as child nodes: The pin mux (selecting pin function

 Direct child nodes of the pin-controller, not a second level?

The child nodes would be direct child nodes.


 While that's quite legal, it means that if you need a particular client
 module to use 4 pins, 2 of which need one samsung,pin-function value and
 2 of which need a different pin-function value, then the client device's
 pinctrl-0 property has to have two entries.

 i.e. a completely hypothetical example roughly based on yours below:

 pinctrl_1: pinctrl@1100 {
 uart0_rxd: uart0-rxd {
 samsung,pins = gpa0-0;
 samsung,pin-function = 2;
 samsung,pin-pud = 0;
 samsung,pin-drv = 0;
 };

 uart0_txd: uart0-txd {
 samsung,pins = gpa0-1;
 samsung,pin-function = 1;
 samsung,pin-pud = 0;
 samsung,pin-drv = 0;
 };
 };

 uart@1380 {
 pinctrl-names = default;
 pinctrl-0 = uart0_rxd uart0_txd;
 };

 rather than:

 pinctrl_1: pinctrl@1100 {
 uart0_opt1: uart0-opt1 {
 uart0_rxd: uart0-rxd {
 samsung,pins = gpa0-0;
 samsung,pin-function = 2;
 samsung,pin-pud = 0;
 samsung,pin-drv = 0;
 };

 uart0_txd: uart0-txd {
 samsung,pins = gpa0-1;
 samsung,pin-function = 1;
 samsung,pin-pud = 0;
 samsung,pin-drv = 0;
 };
 };
 };

 uart@1380 {
 pinctrl-names = default;
 pinctrl-0 = uart0_opt1;
 };

 The latter layout simplifies writing the client nodes, since all the
 related settings can be grouped together by whoever writes the pinctrl
 node, rather than every client author having to work out all the entries
 to include in the list.

 That all said, the way you've defined the binding is perfectly
 legitimate, and I don't have any kind of issue with it; it's just
 something you might want to consider.

Thanks for suggesting this alternate method. I do agree with your
point. But, for now, I would prefer to stabilize this driver without
changing the dt parsing code and make it usable for client nodes. I
will revisit your suggested approach at a later point. I assume for
now that the author's of client nodes know which pin settings to
select.


 Irrespective of whether you choose to keep the binding as-is, or change
 it, please consider it:

 Acked-by: Stephen Warren swar...@wwwdotorg.org

Thanks.


 +  The values specified by these config properties should be dervied from the

 s/dervied/derived/

Ok.