linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] LogiCVC mfd and GPIO support
@ 2019-09-27 10:04 Paul Kocialkowski
  2019-09-27 10:04 ` [PATCH v3 1/5] dt-bindings: Add Xylon vendor prefix Paul Kocialkowski
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Paul Kocialkowski @ 2019-09-27 10:04 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Mark Rutland,
	Lee Jones, Thomas Petazzoni, Paul Kocialkowski

This series introduces support for the LogiCVC GPIO block to the syscon GPIO
driver, with dt bindings documentation also including the top-level mfd
component.

Changes since v2:
- Fixed dt schema examples.

Changes since v1:
- Converted dt bindings documentation to dt schemas;
- Used BIT macro and removed version from structure name;
- Improved documentation example with gpio-line-names;
- Added vendor prefix to dt bindings;
- Added mfd component dt bindings documentation.

Cheers,

Paul


Paul Kocialkowski (5):
  dt-bindings: Add Xylon vendor prefix
  dt-bindings: mfd: Document the Xylon LogiCVC multi-function device
  gpio: syscon: Add support for a custom get operation
  dt-bindings: gpio: Document the Xylon LogiCVC GPIO controller
  gpio: syscon: Add support for the Xylon LogiCVC GPIOs

 .../bindings/gpio/xylon,logicvc-gpio.yaml     | 69 +++++++++++++++++
 .../bindings/mfd/xylon,logicvc.yaml           | 50 +++++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 drivers/gpio/gpio-syscon.c                    | 75 ++++++++++++++++++-
 4 files changed, 193 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/xylon,logicvc-gpio.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml

-- 
2.23.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 1/5] dt-bindings: Add Xylon vendor prefix
  2019-09-27 10:04 [PATCH v3 0/5] LogiCVC mfd and GPIO support Paul Kocialkowski
@ 2019-09-27 10:04 ` Paul Kocialkowski
  2019-09-27 20:49   ` Rob Herring
  2019-09-27 10:04 ` [PATCH v3 2/5] dt-bindings: mfd: Document the Xylon LogiCVC multi-function device Paul Kocialkowski
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Paul Kocialkowski @ 2019-09-27 10:04 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Mark Rutland,
	Lee Jones, Thomas Petazzoni, Paul Kocialkowski

Xylon is an electronics company that produces FPGA hardware block designs
optimized for Xilinx FPGAs.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 29dcc6f8a64a..6c9913244e4c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1036,6 +1036,8 @@ patternProperties:
     description: Xilinx
   "^xunlong,.*":
     description: Shenzhen Xunlong Software CO.,Limited
+  "^xylon,.*":
+    description: Xylon
   "^yones-toptech,.*":
     description: Yones Toptech Co., Ltd.
   "^ysoft,.*":
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 2/5] dt-bindings: mfd: Document the Xylon LogiCVC multi-function device
  2019-09-27 10:04 [PATCH v3 0/5] LogiCVC mfd and GPIO support Paul Kocialkowski
  2019-09-27 10:04 ` [PATCH v3 1/5] dt-bindings: Add Xylon vendor prefix Paul Kocialkowski
@ 2019-09-27 10:04 ` Paul Kocialkowski
  2019-09-27 22:15   ` Rob Herring
  2019-09-27 10:04 ` [PATCH v3 3/5] gpio: syscon: Add support for a custom get operation Paul Kocialkowski
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Paul Kocialkowski @ 2019-09-27 10:04 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Mark Rutland,
	Lee Jones, Thomas Petazzoni, Paul Kocialkowski

The LogiCVC is a display engine which also exposes GPIO functionality.
For this reason, it is described as a multi-function device that is expected
to provide register access to its children nodes for gpio and display.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 .../bindings/mfd/xylon,logicvc.yaml           | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml

diff --git a/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml b/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
new file mode 100644
index 000000000000..abc9937506e0
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 Bootlin
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/mfd/xylon,logicvc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Xylon LogiCVC multi-function device
+
+maintainers:
+  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
+
+description: |
+  The LogiCVC is a display controller that also contains a GPIO controller.
+  As a result, a multi-function device is exposed as parent of the display
+  and GPIO blocks.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - xylon,logicvc-3.02.a
+      - const: syscon
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - xylon,logicvc-3.02.a
+
+  required:
+    - compatible
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    logicvc: logicvc@43c00000 {
+      compatible = "xylon,logicvc-3.02.a", "syscon", "simple-mfd";
+      reg = <0x43c00000 0x6000>;
+      #address-cells = <1>;
+      #size-cells = <1>;
+    };
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 3/5] gpio: syscon: Add support for a custom get operation
  2019-09-27 10:04 [PATCH v3 0/5] LogiCVC mfd and GPIO support Paul Kocialkowski
  2019-09-27 10:04 ` [PATCH v3 1/5] dt-bindings: Add Xylon vendor prefix Paul Kocialkowski
  2019-09-27 10:04 ` [PATCH v3 2/5] dt-bindings: mfd: Document the Xylon LogiCVC multi-function device Paul Kocialkowski
@ 2019-09-27 10:04 ` Paul Kocialkowski
  2019-10-03  8:24   ` Bartosz Golaszewski
  2019-09-27 10:04 ` [PATCH v3 4/5] dt-bindings: gpio: Document the Xylon LogiCVC GPIO controller Paul Kocialkowski
  2019-09-27 10:04 ` [PATCH v3 5/5] gpio: syscon: Add support for the Xylon LogiCVC GPIOs Paul Kocialkowski
  4 siblings, 1 reply; 18+ messages in thread
From: Paul Kocialkowski @ 2019-09-27 10:04 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Mark Rutland,
	Lee Jones, Thomas Petazzoni, Paul Kocialkowski

Some drivers might need a custom get operation to match custom
behavior implemented in the set operation.

Add plumbing for supporting that.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpio/gpio-syscon.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
index 31f332074d7d..05c537ed73f1 100644
--- a/drivers/gpio/gpio-syscon.c
+++ b/drivers/gpio/gpio-syscon.c
@@ -43,8 +43,9 @@ struct syscon_gpio_data {
 	unsigned int	bit_count;
 	unsigned int	dat_bit_offset;
 	unsigned int	dir_bit_offset;
-	void		(*set)(struct gpio_chip *chip,
-			       unsigned offset, int value);
+	int		(*get)(struct gpio_chip *chip, unsigned offset);
+	void		(*set)(struct gpio_chip *chip, unsigned offset,
+			       int value);
 };
 
 struct syscon_gpio_priv {
@@ -252,7 +253,7 @@ static int syscon_gpio_probe(struct platform_device *pdev)
 	priv->chip.label = dev_name(dev);
 	priv->chip.base = -1;
 	priv->chip.ngpio = priv->data->bit_count;
-	priv->chip.get = syscon_gpio_get;
+	priv->chip.get = priv->data->get ? : syscon_gpio_get;
 	if (priv->data->flags & GPIO_SYSCON_FEAT_IN)
 		priv->chip.direction_input = syscon_gpio_dir_in;
 	if (priv->data->flags & GPIO_SYSCON_FEAT_OUT) {
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 4/5] dt-bindings: gpio: Document the Xylon LogiCVC GPIO controller
  2019-09-27 10:04 [PATCH v3 0/5] LogiCVC mfd and GPIO support Paul Kocialkowski
                   ` (2 preceding siblings ...)
  2019-09-27 10:04 ` [PATCH v3 3/5] gpio: syscon: Add support for a custom get operation Paul Kocialkowski
@ 2019-09-27 10:04 ` Paul Kocialkowski
  2019-09-27 22:16   ` Rob Herring
  2019-09-27 10:04 ` [PATCH v3 5/5] gpio: syscon: Add support for the Xylon LogiCVC GPIOs Paul Kocialkowski
  4 siblings, 1 reply; 18+ messages in thread
From: Paul Kocialkowski @ 2019-09-27 10:04 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Mark Rutland,
	Lee Jones, Thomas Petazzoni, Paul Kocialkowski

The Xylon LogiCVC display controller exports some GPIOs, which are
exposed as a separate entity.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 .../bindings/gpio/xylon,logicvc-gpio.yaml     | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/xylon,logicvc-gpio.yaml

diff --git a/Documentation/devicetree/bindings/gpio/xylon,logicvc-gpio.yaml b/Documentation/devicetree/bindings/gpio/xylon,logicvc-gpio.yaml
new file mode 100644
index 000000000000..d102888c1be7
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/xylon,logicvc-gpio.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 Bootlin
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/gpio/xylon,logicvc-gpio.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Xylon LogiCVC GPIO controller
+
+maintainers:
+  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
+
+description: |
+  The LogiCVC GPIO describes the GPIO block included in the LogiCVC display
+  controller. These are meant to be used for controlling display-related
+  signals.
+
+  The controller exposes GPIOs from the display and power control registers,
+  which are mapped by the driver as follows:
+  - GPIO[4:0] (display control) mapped to index 0-4
+  - EN_BLIGHT (power control) mapped to index 5
+  - EN_VDD (power control) mapped to index 6
+  - EN_VEE (power control) mapped to index 7
+  - V_EN (power control) mapped to index 8
+
+properties:
+  $nodename:
+    pattern: "^gpio@[0-9a-f]+$"
+
+  compatible:
+    enum:
+      - xylon,logicvc-3.02.a-gpio
+
+  reg:
+    maxItems: 1
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-controller: true
+
+  gpio-line-names:
+    minItems: 1
+    maxItems: 9
+
+required:
+  - compatible
+  - reg
+  - "#gpio-cells"
+  - gpio-controller
+
+examples:
+  - |
+    logicvc: logicvc@43c00000 {
+      compatible = "xylon,logicvc-3.02.a", "syscon", "simple-mfd";
+      reg = <0x43c00000 0x6000>;
+
+      #address-cells = <1>;
+      #size-cells = <1>;
+
+      logicvc_gpio: gpio@40 {
+        compatible = "xylon,logicvc-3.02.a-gpio";
+        reg = <0x40 0x40>;
+        gpio-controller;
+        #gpio-cells = <2>;
+        gpio-line-names = "GPIO0", "GPIO1", "GPIO2", "GPIO3", "GPIO4",
+               "EN_BLIGHT", "EN_VDD", "EN_VEE", "V_EN";
+      };
+    };
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 5/5] gpio: syscon: Add support for the Xylon LogiCVC GPIOs
  2019-09-27 10:04 [PATCH v3 0/5] LogiCVC mfd and GPIO support Paul Kocialkowski
                   ` (3 preceding siblings ...)
  2019-09-27 10:04 ` [PATCH v3 4/5] dt-bindings: gpio: Document the Xylon LogiCVC GPIO controller Paul Kocialkowski
@ 2019-09-27 10:04 ` Paul Kocialkowski
  2019-10-03  8:26   ` Bartosz Golaszewski
  4 siblings, 1 reply; 18+ messages in thread
From: Paul Kocialkowski @ 2019-09-27 10:04 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Mark Rutland,
	Lee Jones, Thomas Petazzoni, Paul Kocialkowski

The LogiCVC display hardware block comes with GPIO capabilities
that must be exposed separately from the main driver (as GPIOs) for
use with regulators and panels. A syscon is used to share the same
regmap across the two drivers.

Since the GPIO capabilities are pretty simple, add them to the syscon
GPIO driver.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpio/gpio-syscon.c | 68 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
index 05c537ed73f1..cdcb913b8f0c 100644
--- a/drivers/gpio/gpio-syscon.c
+++ b/drivers/gpio/gpio-syscon.c
@@ -190,6 +190,70 @@ static const struct syscon_gpio_data keystone_dsp_gpio = {
 	.set		= keystone_gpio_set,
 };
 
+#define LOGICVC_CTRL_REG		0x40
+#define LOGICVC_CTRL_GPIO_SHIFT		11
+#define LOGICVC_CTRL_GPIO_BITS		5
+
+#define LOGICVC_POWER_CTRL_REG		0x78
+#define LOGICVC_POWER_CTRL_GPIO_SHIFT	0
+#define LOGICVC_POWER_CTRL_GPIO_BITS	4
+
+static void logicvc_gpio_offset(struct syscon_gpio_priv *priv,
+				unsigned offset, unsigned int *reg,
+				unsigned int *bit)
+{
+	if (offset >= LOGICVC_CTRL_GPIO_BITS) {
+		*reg = LOGICVC_POWER_CTRL_REG;
+
+		/* To the (virtual) power ctrl offset. */
+		offset -= LOGICVC_CTRL_GPIO_BITS;
+		/* To the actual bit offset in reg. */
+		offset += LOGICVC_POWER_CTRL_GPIO_SHIFT;
+	} else {
+		*reg = LOGICVC_CTRL_REG;
+
+		/* To the actual bit offset in reg. */
+		offset += LOGICVC_CTRL_GPIO_SHIFT;
+	}
+
+	*bit = BIT(offset);
+}
+
+static int logicvc_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+	struct syscon_gpio_priv *priv = gpiochip_get_data(chip);
+	unsigned int reg;
+	unsigned int bit;
+	unsigned int value;
+	int ret;
+
+	logicvc_gpio_offset(priv, offset, &reg, &bit);
+
+	ret = regmap_read(priv->syscon, reg, &value);
+	if (ret)
+		return ret;
+
+	return !!(value & bit);
+}
+
+static void logicvc_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
+{
+	struct syscon_gpio_priv *priv = gpiochip_get_data(chip);
+	unsigned int reg;
+	unsigned int bit;
+
+	logicvc_gpio_offset(priv, offset, &reg, &bit);
+
+	regmap_update_bits(priv->syscon, reg, bit, val ? bit : 0);
+}
+
+static const struct syscon_gpio_data logicvc_gpio = {
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= LOGICVC_CTRL_GPIO_BITS + LOGICVC_POWER_CTRL_GPIO_BITS,
+	.get		= logicvc_gpio_get,
+	.set		= logicvc_gpio_set,
+};
+
 static const struct of_device_id syscon_gpio_ids[] = {
 	{
 		.compatible	= "cirrus,ep7209-mctrl-gpio",
@@ -203,6 +267,10 @@ static const struct of_device_id syscon_gpio_ids[] = {
 		.compatible	= "rockchip,rk3328-grf-gpio",
 		.data		= &rockchip_rk3328_gpio_mute,
 	},
+	{
+		.compatible	= "xylon,logicvc-3.02.a-gpio",
+		.data		= &logicvc_gpio,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, syscon_gpio_ids);
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 1/5] dt-bindings: Add Xylon vendor prefix
  2019-09-27 10:04 ` [PATCH v3 1/5] dt-bindings: Add Xylon vendor prefix Paul Kocialkowski
@ 2019-09-27 20:49   ` Rob Herring
  0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2019-09-27 20:49 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: linux-gpio, devicetree, linux-kernel, Linus Walleij,
	Bartosz Golaszewski, Mark Rutland, Lee Jones, Thomas Petazzoni,
	Paul Kocialkowski

On Fri, 27 Sep 2019 12:04:03 +0200, Paul Kocialkowski wrote:
> Xylon is an electronics company that produces FPGA hardware block designs
> optimized for Xilinx FPGAs.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/5] dt-bindings: mfd: Document the Xylon LogiCVC multi-function device
  2019-09-27 10:04 ` [PATCH v3 2/5] dt-bindings: mfd: Document the Xylon LogiCVC multi-function device Paul Kocialkowski
@ 2019-09-27 22:15   ` Rob Herring
  2019-10-04 14:45     ` Lee Jones
  0 siblings, 1 reply; 18+ messages in thread
From: Rob Herring @ 2019-09-27 22:15 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: linux-gpio, devicetree, linux-kernel, Linus Walleij,
	Bartosz Golaszewski, Mark Rutland, Lee Jones, Thomas Petazzoni

On Fri, Sep 27, 2019 at 12:04:04PM +0200, Paul Kocialkowski wrote:
> The LogiCVC is a display engine which also exposes GPIO functionality.
> For this reason, it is described as a multi-function device that is expected
> to provide register access to its children nodes for gpio and display.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  .../bindings/mfd/xylon,logicvc.yaml           | 50 +++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml b/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
> new file mode 100644
> index 000000000000..abc9937506e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2019 Bootlin
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/mfd/xylon,logicvc.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Xylon LogiCVC multi-function device
> +
> +maintainers:
> +  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> +
> +description: |
> +  The LogiCVC is a display controller that also contains a GPIO controller.
> +  As a result, a multi-function device is exposed as parent of the display
> +  and GPIO blocks.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - xylon,logicvc-3.02.a
> +      - const: syscon
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - xylon,logicvc-3.02.a

I've seen a couple of these with 'syscon' today, so I fixed the schema 
tool to just exclude 'syscon' and 'simple-mfd' from the generated 
'select'. So you can drop select now.

Reviewed-by: Rob Herring <robh@kernel.org>

> +
> +  required:
> +    - compatible
> +
> +required:
> +  - compatible
> +  - reg
> +
> +examples:
> +  - |
> +    logicvc: logicvc@43c00000 {
> +      compatible = "xylon,logicvc-3.02.a", "syscon", "simple-mfd";
> +      reg = <0x43c00000 0x6000>;
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +    };
> -- 
> 2.23.0
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 4/5] dt-bindings: gpio: Document the Xylon LogiCVC GPIO controller
  2019-09-27 10:04 ` [PATCH v3 4/5] dt-bindings: gpio: Document the Xylon LogiCVC GPIO controller Paul Kocialkowski
@ 2019-09-27 22:16   ` Rob Herring
  0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2019-09-27 22:16 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: linux-gpio, devicetree, linux-kernel, Linus Walleij,
	Bartosz Golaszewski, Mark Rutland, Lee Jones, Thomas Petazzoni,
	Paul Kocialkowski

On Fri, 27 Sep 2019 12:04:06 +0200, Paul Kocialkowski wrote:
> The Xylon LogiCVC display controller exports some GPIOs, which are
> exposed as a separate entity.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  .../bindings/gpio/xylon,logicvc-gpio.yaml     | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/xylon,logicvc-gpio.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/5] gpio: syscon: Add support for a custom get operation
  2019-09-27 10:04 ` [PATCH v3 3/5] gpio: syscon: Add support for a custom get operation Paul Kocialkowski
@ 2019-10-03  8:24   ` Bartosz Golaszewski
  2019-10-03 11:26     ` Paul Kocialkowski
  0 siblings, 1 reply; 18+ messages in thread
From: Bartosz Golaszewski @ 2019-10-03  8:24 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: linux-gpio, linux-devicetree, LKML, Linus Walleij, Rob Herring,
	Mark Rutland, Lee Jones, Thomas Petazzoni

pt., 27 wrz 2019 o 12:04 Paul Kocialkowski
<paul.kocialkowski@bootlin.com> napisał(a):
>
> Some drivers might need a custom get operation to match custom
> behavior implemented in the set operation.
>
> Add plumbing for supporting that.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  drivers/gpio/gpio-syscon.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
> index 31f332074d7d..05c537ed73f1 100644
> --- a/drivers/gpio/gpio-syscon.c
> +++ b/drivers/gpio/gpio-syscon.c
> @@ -43,8 +43,9 @@ struct syscon_gpio_data {
>         unsigned int    bit_count;
>         unsigned int    dat_bit_offset;
>         unsigned int    dir_bit_offset;
> -       void            (*set)(struct gpio_chip *chip,
> -                              unsigned offset, int value);
> +       int             (*get)(struct gpio_chip *chip, unsigned offset);
> +       void            (*set)(struct gpio_chip *chip, unsigned offset,
> +                              int value);

Why did you change this line? Doesn't seem necessary and pollutes the history.

Bart

>  };
>
>  struct syscon_gpio_priv {
> @@ -252,7 +253,7 @@ static int syscon_gpio_probe(struct platform_device *pdev)
>         priv->chip.label = dev_name(dev);
>         priv->chip.base = -1;
>         priv->chip.ngpio = priv->data->bit_count;
> -       priv->chip.get = syscon_gpio_get;
> +       priv->chip.get = priv->data->get ? : syscon_gpio_get;
>         if (priv->data->flags & GPIO_SYSCON_FEAT_IN)
>                 priv->chip.direction_input = syscon_gpio_dir_in;
>         if (priv->data->flags & GPIO_SYSCON_FEAT_OUT) {
> --
> 2.23.0
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 5/5] gpio: syscon: Add support for the Xylon LogiCVC GPIOs
  2019-09-27 10:04 ` [PATCH v3 5/5] gpio: syscon: Add support for the Xylon LogiCVC GPIOs Paul Kocialkowski
@ 2019-10-03  8:26   ` Bartosz Golaszewski
  2019-10-03 11:26     ` Paul Kocialkowski
  0 siblings, 1 reply; 18+ messages in thread
From: Bartosz Golaszewski @ 2019-10-03  8:26 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: linux-gpio, linux-devicetree, LKML, Linus Walleij, Rob Herring,
	Mark Rutland, Lee Jones, Thomas Petazzoni

Hi Paul,

just two nits:

pt., 27 wrz 2019 o 12:04 Paul Kocialkowski
<paul.kocialkowski@bootlin.com> napisał(a):
>
> The LogiCVC display hardware block comes with GPIO capabilities
> that must be exposed separately from the main driver (as GPIOs) for
> use with regulators and panels. A syscon is used to share the same
> regmap across the two drivers.
>
> Since the GPIO capabilities are pretty simple, add them to the syscon
> GPIO driver.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  drivers/gpio/gpio-syscon.c | 68 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>
> diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
> index 05c537ed73f1..cdcb913b8f0c 100644
> --- a/drivers/gpio/gpio-syscon.c
> +++ b/drivers/gpio/gpio-syscon.c
> @@ -190,6 +190,70 @@ static const struct syscon_gpio_data keystone_dsp_gpio = {
>         .set            = keystone_gpio_set,
>  };
>
> +#define LOGICVC_CTRL_REG               0x40
> +#define LOGICVC_CTRL_GPIO_SHIFT                11
> +#define LOGICVC_CTRL_GPIO_BITS         5
> +
> +#define LOGICVC_POWER_CTRL_REG         0x78
> +#define LOGICVC_POWER_CTRL_GPIO_SHIFT  0
> +#define LOGICVC_POWER_CTRL_GPIO_BITS   4
> +
> +static void logicvc_gpio_offset(struct syscon_gpio_priv *priv,
> +                               unsigned offset, unsigned int *reg,
> +                               unsigned int *bit)
> +{
> +       if (offset >= LOGICVC_CTRL_GPIO_BITS) {
> +               *reg = LOGICVC_POWER_CTRL_REG;
> +
> +               /* To the (virtual) power ctrl offset. */
> +               offset -= LOGICVC_CTRL_GPIO_BITS;
> +               /* To the actual bit offset in reg. */
> +               offset += LOGICVC_POWER_CTRL_GPIO_SHIFT;
> +       } else {
> +               *reg = LOGICVC_CTRL_REG;
> +
> +               /* To the actual bit offset in reg. */
> +               offset += LOGICVC_CTRL_GPIO_SHIFT;
> +       }
> +
> +       *bit = BIT(offset);
> +}
> +
> +static int logicvc_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> +       struct syscon_gpio_priv *priv = gpiochip_get_data(chip);
> +       unsigned int reg;
> +       unsigned int bit;
> +       unsigned int value;

Can you put these on a single line?

> +       int ret;
> +
> +       logicvc_gpio_offset(priv, offset, &reg, &bit);
> +
> +       ret = regmap_read(priv->syscon, reg, &value);
> +       if (ret)
> +               return ret;
> +
> +       return !!(value & bit);
> +}
> +
> +static void logicvc_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
> +{
> +       struct syscon_gpio_priv *priv = gpiochip_get_data(chip);
> +       unsigned int reg;
> +       unsigned int bit;

Same here.

Bart

> +
> +       logicvc_gpio_offset(priv, offset, &reg, &bit);
> +
> +       regmap_update_bits(priv->syscon, reg, bit, val ? bit : 0);
> +}
> +
> +static const struct syscon_gpio_data logicvc_gpio = {
> +       .flags          = GPIO_SYSCON_FEAT_OUT,
> +       .bit_count      = LOGICVC_CTRL_GPIO_BITS + LOGICVC_POWER_CTRL_GPIO_BITS,
> +       .get            = logicvc_gpio_get,
> +       .set            = logicvc_gpio_set,
> +};
> +
>  static const struct of_device_id syscon_gpio_ids[] = {
>         {
>                 .compatible     = "cirrus,ep7209-mctrl-gpio",
> @@ -203,6 +267,10 @@ static const struct of_device_id syscon_gpio_ids[] = {
>                 .compatible     = "rockchip,rk3328-grf-gpio",
>                 .data           = &rockchip_rk3328_gpio_mute,
>         },
> +       {
> +               .compatible     = "xylon,logicvc-3.02.a-gpio",
> +               .data           = &logicvc_gpio,
> +       },
>         { }
>  };
>  MODULE_DEVICE_TABLE(of, syscon_gpio_ids);
> --
> 2.23.0
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/5] gpio: syscon: Add support for a custom get operation
  2019-10-03  8:24   ` Bartosz Golaszewski
@ 2019-10-03 11:26     ` Paul Kocialkowski
  2019-10-03 14:05       ` Bartosz Golaszewski
  0 siblings, 1 reply; 18+ messages in thread
From: Paul Kocialkowski @ 2019-10-03 11:26 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: linux-gpio, linux-devicetree, LKML, Linus Walleij, Rob Herring,
	Mark Rutland, Lee Jones, Thomas Petazzoni

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

Hi,

On Thu 03 Oct 19, 10:24, Bartosz Golaszewski wrote:
> pt., 27 wrz 2019 o 12:04 Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> napisał(a):
> >
> > Some drivers might need a custom get operation to match custom
> > behavior implemented in the set operation.
> >
> > Add plumbing for supporting that.
> >
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  drivers/gpio/gpio-syscon.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
> > index 31f332074d7d..05c537ed73f1 100644
> > --- a/drivers/gpio/gpio-syscon.c
> > +++ b/drivers/gpio/gpio-syscon.c
> > @@ -43,8 +43,9 @@ struct syscon_gpio_data {
> >         unsigned int    bit_count;
> >         unsigned int    dat_bit_offset;
> >         unsigned int    dir_bit_offset;
> > -       void            (*set)(struct gpio_chip *chip,
> > -                              unsigned offset, int value);
> > +       int             (*get)(struct gpio_chip *chip, unsigned offset);
> > +       void            (*set)(struct gpio_chip *chip, unsigned offset,
> > +                              int value);
> 
> Why did you change this line? Doesn't seem necessary and pollutes the history.

This is for consistency since both the "chip" and "offset" arguments can fit
in a single line. Since I want the "get" addition to fit in a single line,
bringing back "offset" on the previous line of "set" makes things consistent.
There's probably no particular reason for the split in the first place.

Do you think it needs a separate cosmetic commit only for that?
I'd rather add a note in the commit message and keep the change as-is.

Cheers,

Paul

> Bart
> 
> >  };
> >
> >  struct syscon_gpio_priv {
> > @@ -252,7 +253,7 @@ static int syscon_gpio_probe(struct platform_device *pdev)
> >         priv->chip.label = dev_name(dev);
> >         priv->chip.base = -1;
> >         priv->chip.ngpio = priv->data->bit_count;
> > -       priv->chip.get = syscon_gpio_get;
> > +       priv->chip.get = priv->data->get ? : syscon_gpio_get;
> >         if (priv->data->flags & GPIO_SYSCON_FEAT_IN)
> >                 priv->chip.direction_input = syscon_gpio_dir_in;
> >         if (priv->data->flags & GPIO_SYSCON_FEAT_OUT) {
> > --
> > 2.23.0
> >

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 5/5] gpio: syscon: Add support for the Xylon LogiCVC GPIOs
  2019-10-03  8:26   ` Bartosz Golaszewski
@ 2019-10-03 11:26     ` Paul Kocialkowski
  0 siblings, 0 replies; 18+ messages in thread
From: Paul Kocialkowski @ 2019-10-03 11:26 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: linux-gpio, linux-devicetree, LKML, Linus Walleij, Rob Herring,
	Mark Rutland, Lee Jones, Thomas Petazzoni

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

Hi and thanks for the review!

On Thu 03 Oct 19, 10:26, Bartosz Golaszewski wrote:
> Hi Paul,
> 
> just two nits:
> 
> pt., 27 wrz 2019 o 12:04 Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> napisał(a):
> >
> > The LogiCVC display hardware block comes with GPIO capabilities
> > that must be exposed separately from the main driver (as GPIOs) for
> > use with regulators and panels. A syscon is used to share the same
> > regmap across the two drivers.
> >
> > Since the GPIO capabilities are pretty simple, add them to the syscon
> > GPIO driver.
> >
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  drivers/gpio/gpio-syscon.c | 68 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> >
> > diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
> > index 05c537ed73f1..cdcb913b8f0c 100644
> > --- a/drivers/gpio/gpio-syscon.c
> > +++ b/drivers/gpio/gpio-syscon.c
> > @@ -190,6 +190,70 @@ static const struct syscon_gpio_data keystone_dsp_gpio = {
> >         .set            = keystone_gpio_set,
> >  };
> >
> > +#define LOGICVC_CTRL_REG               0x40
> > +#define LOGICVC_CTRL_GPIO_SHIFT                11
> > +#define LOGICVC_CTRL_GPIO_BITS         5
> > +
> > +#define LOGICVC_POWER_CTRL_REG         0x78
> > +#define LOGICVC_POWER_CTRL_GPIO_SHIFT  0
> > +#define LOGICVC_POWER_CTRL_GPIO_BITS   4
> > +
> > +static void logicvc_gpio_offset(struct syscon_gpio_priv *priv,
> > +                               unsigned offset, unsigned int *reg,
> > +                               unsigned int *bit)
> > +{
> > +       if (offset >= LOGICVC_CTRL_GPIO_BITS) {
> > +               *reg = LOGICVC_POWER_CTRL_REG;
> > +
> > +               /* To the (virtual) power ctrl offset. */
> > +               offset -= LOGICVC_CTRL_GPIO_BITS;
> > +               /* To the actual bit offset in reg. */
> > +               offset += LOGICVC_POWER_CTRL_GPIO_SHIFT;
> > +       } else {
> > +               *reg = LOGICVC_CTRL_REG;
> > +
> > +               /* To the actual bit offset in reg. */
> > +               offset += LOGICVC_CTRL_GPIO_SHIFT;
> > +       }
> > +
> > +       *bit = BIT(offset);
> > +}
> > +
> > +static int logicvc_gpio_get(struct gpio_chip *chip, unsigned offset)
> > +{
> > +       struct syscon_gpio_priv *priv = gpiochip_get_data(chip);
> > +       unsigned int reg;
> > +       unsigned int bit;
> > +       unsigned int value;
> 
> Can you put these on a single line?

Sure thing.

> > +       int ret;
> > +
> > +       logicvc_gpio_offset(priv, offset, &reg, &bit);
> > +
> > +       ret = regmap_read(priv->syscon, reg, &value);
> > +       if (ret)
> > +               return ret;
> > +
> > +       return !!(value & bit);
> > +}
> > +
> > +static void logicvc_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
> > +{
> > +       struct syscon_gpio_priv *priv = gpiochip_get_data(chip);
> > +       unsigned int reg;
> > +       unsigned int bit;
> 
> Same here.

Will do!

Cheers,

Paul

> > +
> > +       logicvc_gpio_offset(priv, offset, &reg, &bit);
> > +
> > +       regmap_update_bits(priv->syscon, reg, bit, val ? bit : 0);
> > +}
> > +
> > +static const struct syscon_gpio_data logicvc_gpio = {
> > +       .flags          = GPIO_SYSCON_FEAT_OUT,
> > +       .bit_count      = LOGICVC_CTRL_GPIO_BITS + LOGICVC_POWER_CTRL_GPIO_BITS,
> > +       .get            = logicvc_gpio_get,
> > +       .set            = logicvc_gpio_set,
> > +};
> > +
> >  static const struct of_device_id syscon_gpio_ids[] = {
> >         {
> >                 .compatible     = "cirrus,ep7209-mctrl-gpio",
> > @@ -203,6 +267,10 @@ static const struct of_device_id syscon_gpio_ids[] = {
> >                 .compatible     = "rockchip,rk3328-grf-gpio",
> >                 .data           = &rockchip_rk3328_gpio_mute,
> >         },
> > +       {
> > +               .compatible     = "xylon,logicvc-3.02.a-gpio",
> > +               .data           = &logicvc_gpio,
> > +       },
> >         { }
> >  };
> >  MODULE_DEVICE_TABLE(of, syscon_gpio_ids);
> > --
> > 2.23.0
> >

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/5] gpio: syscon: Add support for a custom get operation
  2019-10-03 11:26     ` Paul Kocialkowski
@ 2019-10-03 14:05       ` Bartosz Golaszewski
  2019-10-03 14:15         ` Paul Kocialkowski
  0 siblings, 1 reply; 18+ messages in thread
From: Bartosz Golaszewski @ 2019-10-03 14:05 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: linux-gpio, linux-devicetree, LKML, Linus Walleij, Rob Herring,
	Mark Rutland, Lee Jones, Thomas Petazzoni

czw., 3 paź 2019 o 13:26 Paul Kocialkowski
<paul.kocialkowski@bootlin.com> napisał(a):
>
> Hi,
>
> On Thu 03 Oct 19, 10:24, Bartosz Golaszewski wrote:
> > pt., 27 wrz 2019 o 12:04 Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> napisał(a):
> > >
> > > Some drivers might need a custom get operation to match custom
> > > behavior implemented in the set operation.
> > >
> > > Add plumbing for supporting that.
> > >
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > ---
> > >  drivers/gpio/gpio-syscon.c | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
> > > index 31f332074d7d..05c537ed73f1 100644
> > > --- a/drivers/gpio/gpio-syscon.c
> > > +++ b/drivers/gpio/gpio-syscon.c
> > > @@ -43,8 +43,9 @@ struct syscon_gpio_data {
> > >         unsigned int    bit_count;
> > >         unsigned int    dat_bit_offset;
> > >         unsigned int    dir_bit_offset;
> > > -       void            (*set)(struct gpio_chip *chip,
> > > -                              unsigned offset, int value);
> > > +       int             (*get)(struct gpio_chip *chip, unsigned offset);
> > > +       void            (*set)(struct gpio_chip *chip, unsigned offset,
> > > +                              int value);
> >
> > Why did you change this line? Doesn't seem necessary and pollutes the history.
>
> This is for consistency since both the "chip" and "offset" arguments can fit
> in a single line. Since I want the "get" addition to fit in a single line,
> bringing back "offset" on the previous line of "set" makes things consistent.
> There's probably no particular reason for the split in the first place.
>
> Do you think it needs a separate cosmetic commit only for that?
> I'd rather add a note in the commit message and keep the change as-is.
>

The line is still broken - just in a different place. I'd prefer to
leave it as it is frankly, there's nothing wrong with it.

Bart

> Cheers,
>
> Paul
>
> > Bart
> >
> > >  };
> > >
> > >  struct syscon_gpio_priv {
> > > @@ -252,7 +253,7 @@ static int syscon_gpio_probe(struct platform_device *pdev)
> > >         priv->chip.label = dev_name(dev);
> > >         priv->chip.base = -1;
> > >         priv->chip.ngpio = priv->data->bit_count;
> > > -       priv->chip.get = syscon_gpio_get;
> > > +       priv->chip.get = priv->data->get ? : syscon_gpio_get;
> > >         if (priv->data->flags & GPIO_SYSCON_FEAT_IN)
> > >                 priv->chip.direction_input = syscon_gpio_dir_in;
> > >         if (priv->data->flags & GPIO_SYSCON_FEAT_OUT) {
> > > --
> > > 2.23.0
> > >
>
> --
> Paul Kocialkowski, Bootlin
> Embedded Linux and kernel engineering
> https://bootlin.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/5] gpio: syscon: Add support for a custom get operation
  2019-10-03 14:05       ` Bartosz Golaszewski
@ 2019-10-03 14:15         ` Paul Kocialkowski
  0 siblings, 0 replies; 18+ messages in thread
From: Paul Kocialkowski @ 2019-10-03 14:15 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: linux-gpio, linux-devicetree, LKML, Linus Walleij, Rob Herring,
	Mark Rutland, Lee Jones, Thomas Petazzoni

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

Hi,

On Thu 03 Oct 19, 16:05, Bartosz Golaszewski wrote:
> czw., 3 paź 2019 o 13:26 Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> napisał(a):
> >
> > Hi,
> >
> > On Thu 03 Oct 19, 10:24, Bartosz Golaszewski wrote:
> > > pt., 27 wrz 2019 o 12:04 Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.com> napisał(a):
> > > >
> > > > Some drivers might need a custom get operation to match custom
> > > > behavior implemented in the set operation.
> > > >
> > > > Add plumbing for supporting that.
> > > >
> > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > ---
> > > >  drivers/gpio/gpio-syscon.c | 7 ++++---
> > > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
> > > > index 31f332074d7d..05c537ed73f1 100644
> > > > --- a/drivers/gpio/gpio-syscon.c
> > > > +++ b/drivers/gpio/gpio-syscon.c
> > > > @@ -43,8 +43,9 @@ struct syscon_gpio_data {
> > > >         unsigned int    bit_count;
> > > >         unsigned int    dat_bit_offset;
> > > >         unsigned int    dir_bit_offset;
> > > > -       void            (*set)(struct gpio_chip *chip,
> > > > -                              unsigned offset, int value);
> > > > +       int             (*get)(struct gpio_chip *chip, unsigned offset);
> > > > +       void            (*set)(struct gpio_chip *chip, unsigned offset,
> > > > +                              int value);
> > >
> > > Why did you change this line? Doesn't seem necessary and pollutes the history.
> >
> > This is for consistency since both the "chip" and "offset" arguments can fit
> > in a single line. Since I want the "get" addition to fit in a single line,
> > bringing back "offset" on the previous line of "set" makes things consistent.
> > There's probably no particular reason for the split in the first place.
> >
> > Do you think it needs a separate cosmetic commit only for that?
> > I'd rather add a note in the commit message and keep the change as-is.
> >
> 
> The line is still broken - just in a different place. I'd prefer to
> leave it as it is frankly, there's nothing wrong with it.

The point is rather that this introduces inconsistency between the two lines.
It's definitely not a major issue, but I still believe it is a coding style
issue. It surely doesn't hurt to fix it.

Cheers,

Paul

> Bart
> 
> > Cheers,
> >
> > Paul
> >
> > > Bart
> > >
> > > >  };
> > > >
> > > >  struct syscon_gpio_priv {
> > > > @@ -252,7 +253,7 @@ static int syscon_gpio_probe(struct platform_device *pdev)
> > > >         priv->chip.label = dev_name(dev);
> > > >         priv->chip.base = -1;
> > > >         priv->chip.ngpio = priv->data->bit_count;
> > > > -       priv->chip.get = syscon_gpio_get;
> > > > +       priv->chip.get = priv->data->get ? : syscon_gpio_get;
> > > >         if (priv->data->flags & GPIO_SYSCON_FEAT_IN)
> > > >                 priv->chip.direction_input = syscon_gpio_dir_in;
> > > >         if (priv->data->flags & GPIO_SYSCON_FEAT_OUT) {
> > > > --
> > > > 2.23.0
> > > >
> >
> > --
> > Paul Kocialkowski, Bootlin
> > Embedded Linux and kernel engineering
> > https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/5] dt-bindings: mfd: Document the Xylon LogiCVC multi-function device
  2019-09-27 22:15   ` Rob Herring
@ 2019-10-04 14:45     ` Lee Jones
  2019-10-04 16:09       ` Rob Herring
  0 siblings, 1 reply; 18+ messages in thread
From: Lee Jones @ 2019-10-04 14:45 UTC (permalink / raw)
  To: Rob Herring
  Cc: Paul Kocialkowski, linux-gpio, devicetree, linux-kernel,
	Linus Walleij, Bartosz Golaszewski, Mark Rutland,
	Thomas Petazzoni

On Fri, 27 Sep 2019, Rob Herring wrote:

> On Fri, Sep 27, 2019 at 12:04:04PM +0200, Paul Kocialkowski wrote:
> > The LogiCVC is a display engine which also exposes GPIO functionality.
> > For this reason, it is described as a multi-function device that is expected
> > to provide register access to its children nodes for gpio and display.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  .../bindings/mfd/xylon,logicvc.yaml           | 50 +++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml b/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
> > new file mode 100644
> > index 000000000000..abc9937506e0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
> > @@ -0,0 +1,50 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright 2019 Bootlin
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/mfd/xylon,logicvc.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: Xylon LogiCVC multi-function device
> > +
> > +maintainers:
> > +  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > +
> > +description: |
> > +  The LogiCVC is a display controller that also contains a GPIO controller.
> > +  As a result, a multi-function device is exposed as parent of the display
> > +  and GPIO blocks.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - xylon,logicvc-3.02.a
> > +      - const: syscon
> > +      - const: simple-mfd
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +select:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - xylon,logicvc-3.02.a
> 
> I've seen a couple of these with 'syscon' today, so I fixed the schema 
> tool to just exclude 'syscon' and 'simple-mfd' from the generated 
> 'select'. So you can drop select now.

Does this need to happen before this patch can be applied?

> Reviewed-by: Rob Herring <robh@kernel.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/5] dt-bindings: mfd: Document the Xylon LogiCVC multi-function device
  2019-10-04 14:45     ` Lee Jones
@ 2019-10-04 16:09       ` Rob Herring
  2019-10-04 18:12         ` Paul Kocialkowski
  0 siblings, 1 reply; 18+ messages in thread
From: Rob Herring @ 2019-10-04 16:09 UTC (permalink / raw)
  To: Lee Jones
  Cc: Paul Kocialkowski, open list:GPIO SUBSYSTEM, devicetree,
	linux-kernel, Linus Walleij, Bartosz Golaszewski, Mark Rutland,
	Thomas Petazzoni

On Fri, Oct 4, 2019 at 9:45 AM Lee Jones <lee.jones@linaro.org> wrote:
>
> On Fri, 27 Sep 2019, Rob Herring wrote:
>
> > On Fri, Sep 27, 2019 at 12:04:04PM +0200, Paul Kocialkowski wrote:
> > > The LogiCVC is a display engine which also exposes GPIO functionality.
> > > For this reason, it is described as a multi-function device that is expected
> > > to provide register access to its children nodes for gpio and display.
> > >
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > ---
> > >  .../bindings/mfd/xylon,logicvc.yaml           | 50 +++++++++++++++++++
> > >  1 file changed, 50 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml b/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
> > > new file mode 100644
> > > index 000000000000..abc9937506e0
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
> > > @@ -0,0 +1,50 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +# Copyright 2019 Bootlin
> > > +%YAML 1.2
> > > +---
> > > +$id: "http://devicetree.org/schemas/mfd/xylon,logicvc.yaml#"
> > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > > +
> > > +title: Xylon LogiCVC multi-function device
> > > +
> > > +maintainers:
> > > +  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > +
> > > +description: |
> > > +  The LogiCVC is a display controller that also contains a GPIO controller.
> > > +  As a result, a multi-function device is exposed as parent of the display
> > > +  and GPIO blocks.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - enum:
> > > +          - xylon,logicvc-3.02.a
> > > +      - const: syscon
> > > +      - const: simple-mfd
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +select:
> > > +  properties:
> > > +    compatible:
> > > +      contains:
> > > +        enum:
> > > +          - xylon,logicvc-3.02.a
> >
> > I've seen a couple of these with 'syscon' today, so I fixed the schema
> > tool to just exclude 'syscon' and 'simple-mfd' from the generated
> > 'select'. So you can drop select now.
>
> Does this need to happen before this patch can be applied?

Drop the 'select'? Yes that should happen first.

Rob

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/5] dt-bindings: mfd: Document the Xylon LogiCVC multi-function device
  2019-10-04 16:09       ` Rob Herring
@ 2019-10-04 18:12         ` Paul Kocialkowski
  0 siblings, 0 replies; 18+ messages in thread
From: Paul Kocialkowski @ 2019-10-04 18:12 UTC (permalink / raw)
  To: Rob Herring
  Cc: Lee Jones, open list:GPIO SUBSYSTEM, devicetree, linux-kernel,
	Linus Walleij, Bartosz Golaszewski, Mark Rutland,
	Thomas Petazzoni

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

Hi,

On Fri 04 Oct 19, 11:09, Rob Herring wrote:
> On Fri, Oct 4, 2019 at 9:45 AM Lee Jones <lee.jones@linaro.org> wrote:
> >
> > On Fri, 27 Sep 2019, Rob Herring wrote:
> >
> > > On Fri, Sep 27, 2019 at 12:04:04PM +0200, Paul Kocialkowski wrote:
> > > > The LogiCVC is a display engine which also exposes GPIO functionality.
> > > > For this reason, it is described as a multi-function device that is expected
> > > > to provide register access to its children nodes for gpio and display.
> > > >
> > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > ---
> > > >  .../bindings/mfd/xylon,logicvc.yaml           | 50 +++++++++++++++++++
> > > >  1 file changed, 50 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml b/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
> > > > new file mode 100644
> > > > index 000000000000..abc9937506e0
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/mfd/xylon,logicvc.yaml
> > > > @@ -0,0 +1,50 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +# Copyright 2019 Bootlin
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: "http://devicetree.org/schemas/mfd/xylon,logicvc.yaml#"
> > > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > > > +
> > > > +title: Xylon LogiCVC multi-function device
> > > > +
> > > > +maintainers:
> > > > +  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > +
> > > > +description: |
> > > > +  The LogiCVC is a display controller that also contains a GPIO controller.
> > > > +  As a result, a multi-function device is exposed as parent of the display
> > > > +  and GPIO blocks.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    items:
> > > > +      - enum:
> > > > +          - xylon,logicvc-3.02.a
> > > > +      - const: syscon
> > > > +      - const: simple-mfd
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +select:
> > > > +  properties:
> > > > +    compatible:
> > > > +      contains:
> > > > +        enum:
> > > > +          - xylon,logicvc-3.02.a
> > >
> > > I've seen a couple of these with 'syscon' today, so I fixed the schema
> > > tool to just exclude 'syscon' and 'simple-mfd' from the generated
> > > 'select'. So you can drop select now.
> >
> > Does this need to happen before this patch can be applied?
> 
> Drop the 'select'? Yes that should happen first.

I'll definitely respin the series for a v3 soon so no worries, I'll get rid
of it.

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2019-10-04 18:12 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-27 10:04 [PATCH v3 0/5] LogiCVC mfd and GPIO support Paul Kocialkowski
2019-09-27 10:04 ` [PATCH v3 1/5] dt-bindings: Add Xylon vendor prefix Paul Kocialkowski
2019-09-27 20:49   ` Rob Herring
2019-09-27 10:04 ` [PATCH v3 2/5] dt-bindings: mfd: Document the Xylon LogiCVC multi-function device Paul Kocialkowski
2019-09-27 22:15   ` Rob Herring
2019-10-04 14:45     ` Lee Jones
2019-10-04 16:09       ` Rob Herring
2019-10-04 18:12         ` Paul Kocialkowski
2019-09-27 10:04 ` [PATCH v3 3/5] gpio: syscon: Add support for a custom get operation Paul Kocialkowski
2019-10-03  8:24   ` Bartosz Golaszewski
2019-10-03 11:26     ` Paul Kocialkowski
2019-10-03 14:05       ` Bartosz Golaszewski
2019-10-03 14:15         ` Paul Kocialkowski
2019-09-27 10:04 ` [PATCH v3 4/5] dt-bindings: gpio: Document the Xylon LogiCVC GPIO controller Paul Kocialkowski
2019-09-27 22:16   ` Rob Herring
2019-09-27 10:04 ` [PATCH v3 5/5] gpio: syscon: Add support for the Xylon LogiCVC GPIOs Paul Kocialkowski
2019-10-03  8:26   ` Bartosz Golaszewski
2019-10-03 11:26     ` Paul Kocialkowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).