linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 00/13] Add support for Kontron sl28cpld
@ 2020-07-25 23:18 Michael Walle
  2020-07-25 23:18 ` [PATCH v6 01/13] mfd: add simple regmap based I2C driver Michael Walle
                   ` (12 more replies)
  0 siblings, 13 replies; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle

The Kontron sl28cpld is a board management chip providing gpio, pwm, fan
monitoring and an interrupt controller. For now this controller is used on
the Kontron SMARC-sAL28 board. But because of its flexible nature, it
might also be used on other boards in the future. The individual blocks
(like gpio, pwm, etc) are kept intentionally small. The MFD core driver
then instantiates different (or multiple of the same) blocks. It also
provides the register layout so it might be updated in the future without a
device tree change; and support other boards with a different layout or
functionalities.

See also [1] for more information.

This is my first take of a MFD driver. I don't know whether the subsystem
maintainers should only be CCed on the patches which affect the subsystem
or on all patches for this series. I've chosen the latter so you can get a
more complete picture.

[1] https://lore.kernel.org/linux-devicetree/0e3e8204ab992d75aa07fc36af7e4ab2@walle.cc/

Changes for v4 and above are tracked in the patches, suggested by Lee.

Changes since v3:
 - use of_platform_populate() to populate internal devices using the
   internal register offsets as unit-addresses
 - because we don't use mfd_cells anymore, we cannot use IORESOURCE_REG,
   but instead parse the reg property in each individual driver
 - dropped the following patches because they were already merged:
     gpiolib: Introduce gpiochip_irqchip_add_domain()
     gpio: add a reusable generic gpio_chip using regmap
 - dropped the following patches because they are no longer needed:
     include/linux/ioport.h: add helper to define REG resource constructs
     mfd: mfd-core: Don't overwrite the dma_mask of the child device
     mfd: mfd-core: match device tree node against reg property
 - rephrase commit messages, as suggested by Thomas Gleixner

Changes since v2:
As suggested by Guenter Roeck:
 - added sl28cpld.rst to index.rst
 - removed sl28cpld_wdt_status()
 - reverse christmas tree local variable ordering
 - assign device_property_read_bool() retval directly
 - introduce WDT_DEFAULT_TIMEOUT and use it if the hardware reports
   0 as timeout.
 - set WDOG_HW_RUNNING if applicable
 - remove platform_set_drvdata() leftover

As suggested by Bartosz Golaszewski:
 - don't export gpio_regmap_simple_xlate()
 - combine local variable declaration of the same type
 - drop the "struct gpio_regmap_addr", instead use -1 to force an address
   offset of zero
 - fix typo
 - use "struct gpio_regmap_config" pattern, keep "struct gpio_regmap"
   private. this also means we need a getter/setter for the driver_data
   element.

As suggested by Linus Walleij:
 - don't store irq_domain in gpio-regmap. drop to_irq() altogether for now.
   Instead there is now a new patch which lets us set the irqdomain of the
   gpiochip_irqchip and use its .to_irq() function. This way we don't have
   to expose the gpio_chip inside the gpio-regmap to the user.

Changes since v1:
 - use of_match_table in all drivers, needed for automatic module loading,
   when using OF_MFD_CELL()
 - add new gpio-regmap.c which adds a generic regmap gpio_chip
   implementation
 - new patch for reqmap_irq, so we can reuse its implementation
 - remove almost any code from gpio-sl28cpld.c, instead use gpio-regmap and
   regmap-irq
 - change the handling of the mfd core vs device tree nodes; add a new
   property "of_reg" to the mfd_cell struct which, when set, is matched to
   the unit-address of the device tree nodes.
 - fix sl28cpld watchdog when it is not initialized by the bootloader.
   Explicitly set the operation mode.
 - also add support for kontron,assert-wdt-timeout-pin in sl28cpld-wdt.

As suggested by Bartosz Golaszewski:
 - define registers as hex
 - make gpio enum uppercase
 - move parent regmap check before memory allocation
 - use device_property_read_bool() instead of the of_ version
 - mention the gpio flavors in the bindings documentation

As suggested by Guenter Roeck:
 - cleanup #includes and sort them
 - use devm_watchdog_register_device()
 - use watchdog_stop_on_reboot()
 - provide a Documentation/hwmon/sl28cpld.rst
 - cleaned up the weird tristate->bool and I2C=y issue. Instead mention
   that the MFD driver is bool because of the following intc patch
 - removed the SL28CPLD_IRQ typo

As suggested by Rob Herring:
 - combine all dt bindings docs into one patch
 - change the node name for all gpio flavors to "gpio"
 - removed the interrupts-extended rule
 - cleaned up the unit-address space, see above

Michael Walle (13):
  mfd: add simple regmap based I2C driver
  dt-bindings: mfd: Add bindings for sl28cpld
  mfd: simple-mfd-i2c: add sl28cpld support
  irqchip: add sl28cpld interrupt controller support
  watchdog: add support for sl28cpld watchdog
  pwm: add support for sl28cpld PWM controller
  gpio: add support for the sl28cpld GPIO controller
  hwmon: add support for the sl28cpld hardware monitoring controller
  arm64: dts: freescale: sl28: enable sl28cpld
  arm64: dts: freescale: sl28: map GPIOs to input events
  arm64: dts: freescale: sl28: enable LED support
  arm64: dts: freescale: sl28: enable fan support
  arm64: defconfig: enable the sl28cpld board management controller

 .../bindings/gpio/kontron,sl28cpld-gpio.yaml  |  54 +++++
 .../hwmon/kontron,sl28cpld-hwmon.yaml         |  27 +++
 .../kontron,sl28cpld-intc.yaml                |  54 +++++
 .../bindings/mfd/kontron,sl28cpld.yaml        | 153 ++++++++++++
 .../bindings/pwm/kontron,sl28cpld-pwm.yaml    |  35 +++
 .../watchdog/kontron,sl28cpld-wdt.yaml        |  35 +++
 Documentation/hwmon/index.rst                 |   1 +
 Documentation/hwmon/sl28cpld.rst              |  36 +++
 .../fsl-ls1028a-kontron-kbox-a-230-ls.dts     |  14 ++
 .../fsl-ls1028a-kontron-sl28-var3-ads2.dts    |   9 +
 .../freescale/fsl-ls1028a-kontron-sl28.dts    | 134 ++++++++++
 arch/arm64/configs/defconfig                  |   5 +
 drivers/gpio/Kconfig                          |  12 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-sl28cpld.c                  | 161 ++++++++++++
 drivers/hwmon/Kconfig                         |  10 +
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/sl28cpld-hwmon.c                | 142 +++++++++++
 drivers/irqchip/Kconfig                       |   8 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-sl28cpld.c                |  96 ++++++++
 drivers/mfd/Kconfig                           |   5 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/simple-mfd-i2c.c                  |  56 +++++
 drivers/pwm/Kconfig                           |  10 +
 drivers/pwm/Makefile                          |   1 +
 drivers/pwm/pwm-sl28cpld.c                    | 223 +++++++++++++++++
 drivers/watchdog/Kconfig                      |  11 +
 drivers/watchdog/Makefile                     |   1 +
 drivers/watchdog/sl28cpld_wdt.c               | 229 ++++++++++++++++++
 30 files changed, 1526 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
 create mode 100644 Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
 create mode 100644 Documentation/devicetree/bindings/pwm/kontron,sl28cpld-pwm.yaml
 create mode 100644 Documentation/devicetree/bindings/watchdog/kontron,sl28cpld-wdt.yaml
 create mode 100644 Documentation/hwmon/sl28cpld.rst
 create mode 100644 drivers/gpio/gpio-sl28cpld.c
 create mode 100644 drivers/hwmon/sl28cpld-hwmon.c
 create mode 100644 drivers/irqchip/irq-sl28cpld.c
 create mode 100644 drivers/mfd/simple-mfd-i2c.c
 create mode 100644 drivers/pwm/pwm-sl28cpld.c
 create mode 100644 drivers/watchdog/sl28cpld_wdt.c

-- 
2.20.1


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

* [PATCH v6 01/13] mfd: add simple regmap based I2C driver
  2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
@ 2020-07-25 23:18 ` Michael Walle
  2020-07-28  7:19   ` Lee Jones
  2020-07-25 23:18 ` [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld Michael Walle
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle

There are I2C devices which contain several different functions but
doesn't require any special access functions. For these kind of drivers
an I2C regmap should be enough.

Create an I2C driver which creates an I2C regmap and enumerates its
children. If a device wants to use this as its MFD core driver, it has
to add an individual compatible string. It may provide its own regmap
configuration.

Subdevices can use dev_get_regmap() on the parent to get their regmap
instance.

Signed-off-by: Michael Walle <michael@walle.cc>
---
Changes since v5:
 - removed "select MFD_CORE" in Kconfig
 - removed help text in Kconfig, we assume that the users of this
   driver will have a "select MFD_SIMPLE_MFD_I2C". Instead added
   a small description to the driver itself.
 - removed "struct simple_mfd_i2c_config" and use regmap_config
   directly
 - changed builtin_i2c_driver() to module_i2c_driver(), added
   MODULE_ boilerplate
 - cleaned up the included files

Changes since v4:
 - new patch. Lee, please bear with me. I didn't want to delay the
   new version (where a lot of remarks on the other patches were
   addressed) even more, just because we haven't figured out how
   to deal with the MFD part. So for now, I've included this one.

 drivers/mfd/Kconfig          |  5 ++++
 drivers/mfd/Makefile         |  1 +
 drivers/mfd/simple-mfd-i2c.c | 55 ++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+)
 create mode 100644 drivers/mfd/simple-mfd-i2c.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 33df0837ab41..c08539c7a166 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1162,6 +1162,11 @@ config MFD_SI476X_CORE
 	  To compile this driver as a module, choose M here: the
 	  module will be called si476x-core.
 
+config MFD_SIMPLE_MFD_I2C
+	tristate
+	depends on I2C
+	select REGMAP_I2C
+
 config MFD_SM501
 	tristate "Silicon Motion SM501"
 	depends on HAS_DMA
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index a60e5f835283..78d24a3e7c9e 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -264,3 +264,4 @@ obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
 obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
 
 obj-$(CONFIG_SGI_MFD_IOC3)	+= ioc3.o
+obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)	+= simple-mfd-i2c.o
diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
new file mode 100644
index 000000000000..45090ddad104
--- /dev/null
+++ b/drivers/mfd/simple-mfd-i2c.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * A very simple I2C MFD driver
+ *
+ * The driver enumerates its children and registers a common regmap. Use
+ * dev_get_regmap(pdev->dev.parent, NULL) in the child nodes to fetch that
+ * regmap instance.
+ *
+ * In the future this driver might be extended to support also other interfaces
+ * like SPI etc.
+ */
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/regmap.h>
+
+static const struct regmap_config simple_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static int simple_mfd_i2c_probe(struct i2c_client *i2c)
+{
+	const struct regmap_config *config;
+	struct regmap *regmap;
+
+	config = device_get_match_data(&i2c->dev);
+	if (!config)
+		config = &simple_regmap_config;
+
+	regmap = devm_regmap_init_i2c(i2c, config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return devm_of_platform_populate(&i2c->dev);
+}
+
+static const struct of_device_id simple_mfd_i2c_of_match[] = {
+	{}
+};
+MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
+
+static struct i2c_driver simple_mfd_i2c_driver = {
+	.probe_new = simple_mfd_i2c_probe,
+	.driver = {
+		.name = "simple-mfd-i2c",
+		.of_match_table = simple_mfd_i2c_of_match,
+	},
+};
+module_i2c_driver(simple_mfd_i2c_driver);
+
+MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
+MODULE_DESCRIPTION("Simple I2C MFD driver");
+MODULE_LICENSE("GPL v2");
-- 
2.20.1


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

* [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld
  2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
  2020-07-25 23:18 ` [PATCH v6 01/13] mfd: add simple regmap based I2C driver Michael Walle
@ 2020-07-25 23:18 ` Michael Walle
  2020-07-28  7:24   ` Lee Jones
  2020-07-25 23:18 ` [PATCH v6 03/13] mfd: simple-mfd-i2c: add sl28cpld support Michael Walle
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle, Rob Herring

Add a device tree bindings for the board management controller found on
the Kontron SMARC-sAL28 board.

Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes since v5:
 - none

Changes since v4:
 - fix the regex of the unit-address

Changes since v3:
 - see cover letter

 .../bindings/gpio/kontron,sl28cpld-gpio.yaml  |  54 +++++++
 .../hwmon/kontron,sl28cpld-hwmon.yaml         |  27 ++++
 .../kontron,sl28cpld-intc.yaml                |  54 +++++++
 .../bindings/mfd/kontron,sl28cpld.yaml        | 153 ++++++++++++++++++
 .../bindings/pwm/kontron,sl28cpld-pwm.yaml    |  35 ++++
 .../watchdog/kontron,sl28cpld-wdt.yaml        |  35 ++++
 6 files changed, 358 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
 create mode 100644 Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
 create mode 100644 Documentation/devicetree/bindings/pwm/kontron,sl28cpld-pwm.yaml
 create mode 100644 Documentation/devicetree/bindings/watchdog/kontron,sl28cpld-wdt.yaml

diff --git a/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml b/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
new file mode 100644
index 000000000000..9a63a158a796
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/kontron,sl28cpld-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPIO driver for the sl28cpld board management controller
+
+maintainers:
+  - Michael Walle <michael@walle.cc>
+
+description: |
+  This module is part of the sl28cpld multi-function device. For more
+  details see Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
+
+  There are three flavors of the GPIO controller, one full featured
+  input/output with interrupt support (kontron,sl28cpld-gpio), one
+  output-only (kontron,sl28-gpo) and one input-only (kontron,sl28-gpi).
+
+  Each controller supports 8 GPIO lines.
+
+properties:
+  compatible:
+    enum:
+      - kontron,sl28cpld-gpio
+      - kontron,sl28cpld-gpi
+      - kontron,sl28cpld-gpo
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  "#interrupt-cells":
+    const: 2
+
+  interrupt-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-controller: true
+
+  gpio-line-names:
+      minItems: 1
+      maxItems: 8
+
+required:
+  - compatible
+  - "#gpio-cells"
+  - gpio-controller
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
new file mode 100644
index 000000000000..1cebd61c6c32
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/kontron,sl28cpld-hwmon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Hardware monitoring driver for the sl28cpld board management controller
+
+maintainers:
+  - Michael Walle <michael@walle.cc>
+
+description: |
+  This module is part of the sl28cpld multi-function device. For more
+  details see Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
+
+properties:
+  compatible:
+    enum:
+      - kontron,sl28cpld-fan
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
new file mode 100644
index 000000000000..4c39e9ff9aea
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/kontron,sl28cpld-intc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Interrupt controller driver for the sl28cpld board management controller
+
+maintainers:
+  - Michael Walle <michael@walle.cc>
+
+description: |
+  This module is part of the sl28cpld multi-function device. For more
+  details see Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
+
+  The following interrupts are available. All types and levels are fixed
+  and handled by the board management controller.
+
+  ==== ============= ==================================
+   IRQ line/device   description
+  ==== ============= ==================================
+    0  RTC_INT#      Interrupt line from on-board RTC
+    1  SMB_ALERT#    Event on SMB_ALERT# line (P1)
+    2  ESPI_ALERT0#  Event on ESPI_ALERT0# line (S43)
+    3  ESPI_ALERT1#  Event on ESPI_ALERT1# line (S44)
+    4  PWR_BTN#      Event on PWR_BTN# line (P128)
+    5  SLEEP#        Event on SLEEP# line (S149)
+    6  watchdog      Interrupt of the internal watchdog
+    7  n/a           not used
+  ==== ============= ==================================
+
+properties:
+  compatible:
+    enum:
+      - kontron,sl28cpld-intc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  "#interrupt-cells":
+    const: 2
+
+  interrupt-controller: true
+
+required:
+  - compatible
+  - interrupts
+  - "#interrupt-cells"
+  - interrupt-controller
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml b/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
new file mode 100644
index 000000000000..e3a62db678e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
@@ -0,0 +1,153 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/kontron,sl28cpld.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Kontron's sl28cpld board management controller
+
+maintainers:
+  - Michael Walle <michael@walle.cc>
+
+description: |
+  The board management controller may contain different IP blocks like
+  watchdog, fan monitoring, PWM controller, interrupt controller and a
+  GPIO controller.
+
+properties:
+  compatible:
+    const: kontron,sl28cpld-r1
+
+  reg:
+    description:
+      I2C device address.
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  "#interrupt-cells":
+    const: 2
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller: true
+
+patternProperties:
+  "^gpio(@[0-9a-f]+)?$":
+    $ref: ../gpio/kontron,sl28cpld-gpio.yaml
+
+  "^hwmon(@[0-9a-f]+)?$":
+    $ref: ../hwmon/kontron,sl28cpld-hwmon.yaml
+
+  "^interrupt-controller(@[0-9a-f]+)?$":
+    $ref: ../interrupt-controller/kontron,sl28cpld-intc.yaml
+
+  "^pwm(@[0-9a-f]+)?$":
+    $ref: ../pwm/kontron,sl28cpld-pwm.yaml
+
+  "^watchdog(@[0-9a-f]+)?$":
+    $ref: ../watchdog/kontron,sl28cpld-wdt.yaml
+
+required:
+  - "#address-cells"
+  - "#size-cells"
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        sl28cpld@4a {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            compatible = "kontron,sl28cpld-r1";
+            reg = <0x4a>;
+
+            watchdog@4 {
+                compatible = "kontron,sl28cpld-wdt";
+                reg = <0x4>;
+                kontron,assert-wdt-timeout-pin;
+            };
+
+            hwmon@b {
+                compatible = "kontron,sl28cpld-fan";
+                reg = <0xb>;
+            };
+
+            pwm@c {
+                #pwm-cells = <2>;
+                compatible = "kontron,sl28cpld-pwm";
+                reg = <0xc>;
+            };
+
+            pwm@e {
+                #pwm-cells = <2>;
+                compatible = "kontron,sl28cpld-pwm";
+                reg = <0xe>;
+            };
+
+            gpio@10 {
+                compatible = "kontron,sl28cpld-gpio";
+                reg = <0x10>;
+                interrupts-extended = <&gpio2 6
+                               IRQ_TYPE_EDGE_FALLING>;
+
+                gpio-controller;
+                #gpio-cells = <2>;
+                gpio-line-names = "a", "b", "c";
+
+                interrupt-controller;
+                #interrupt-cells = <2>;
+            };
+
+            gpio@15 {
+                compatible = "kontron,sl28cpld-gpio";
+                reg = <0x15>;
+                interrupts-extended = <&gpio2 6
+                               IRQ_TYPE_EDGE_FALLING>;
+
+                gpio-controller;
+                #gpio-cells = <2>;
+
+                interrupt-controller;
+                #interrupt-cells = <2>;
+            };
+
+            gpio@1a {
+                compatible = "kontron,sl28cpld-gpo";
+                reg = <0x1a>;
+
+                gpio-controller;
+                #gpio-cells = <2>;
+            };
+
+            gpio@1b {
+                compatible = "kontron,sl28cpld-gpi";
+                reg = <0x1b>;
+
+                gpio-controller;
+                #gpio-cells = <2>;
+            };
+
+            interrupt-controller@1c {
+                compatible = "kontron,sl28cpld-intc";
+                reg = <0x1c>;
+                interrupts-extended = <&gpio2 6
+                               IRQ_TYPE_EDGE_FALLING>;
+
+                interrupt-controller;
+                #interrupt-cells = <2>;
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/pwm/kontron,sl28cpld-pwm.yaml b/Documentation/devicetree/bindings/pwm/kontron,sl28cpld-pwm.yaml
new file mode 100644
index 000000000000..02fe88c30233
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/kontron,sl28cpld-pwm.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/kontron,sl28cpld-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: PWM driver for the sl28cpld board management controller
+
+maintainers:
+  - Michael Walle <michael@walle.cc>
+
+description: |
+  This module is part of the sl28cpld multi-function device. For more
+  details see Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
+
+  The controller supports one PWM channel and supports only four distinct
+  frequencies (250Hz, 500Hz, 1kHz, 2kHz).
+
+allOf:
+  - $ref: pwm.yaml#
+
+properties:
+  compatible:
+    const: kontron,sl28cpld-pwm
+
+  reg:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 2
+
+required:
+  - compatible
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/watchdog/kontron,sl28cpld-wdt.yaml b/Documentation/devicetree/bindings/watchdog/kontron,sl28cpld-wdt.yaml
new file mode 100644
index 000000000000..dd6559f2973a
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/kontron,sl28cpld-wdt.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/kontron,sl28cpld-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Watchdog driver for the sl28cpld board management controller
+
+maintainers:
+  - Michael Walle <michael@walle.cc>
+
+description: |
+  This module is part of the sl28cpld multi-function device. For more
+  details see Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
+
+allOf:
+  - $ref: watchdog.yaml#
+
+properties:
+  compatible:
+    const: kontron,sl28cpld-wdt
+
+  reg:
+    maxItems: 1
+
+  kontron,assert-wdt-timeout-pin:
+    description: The SMARC standard defines a WDT_TIME_OUT# pin. If this
+      property is set, this output will be pulsed when the watchdog bites
+      and the system resets.
+    type: boolean
+
+required:
+  - compatible
+
+additionalProperties: false
-- 
2.20.1


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

* [PATCH v6 03/13] mfd: simple-mfd-i2c: add sl28cpld support
  2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
  2020-07-25 23:18 ` [PATCH v6 01/13] mfd: add simple regmap based I2C driver Michael Walle
  2020-07-25 23:18 ` [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld Michael Walle
@ 2020-07-25 23:18 ` Michael Walle
  2020-07-28  7:25   ` Lee Jones
  2020-07-25 23:18 ` [PATCH v6 04/13] irqchip: add sl28cpld interrupt controller support Michael Walle
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle

Add the core support for the board management controller found on the
SMARC-sAL28 board.

At the moment, this controller is used on the Kontron SMARC-sAL28 board.

Signed-off-by: Michael Walle <michael@walle.cc>
---
Changes since v5:
 - none

Changes since v4:
 - new patch

 drivers/mfd/simple-mfd-i2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
index 45090ddad104..dd19abd41d30 100644
--- a/drivers/mfd/simple-mfd-i2c.c
+++ b/drivers/mfd/simple-mfd-i2c.c
@@ -37,6 +37,7 @@ static int simple_mfd_i2c_probe(struct i2c_client *i2c)
 }
 
 static const struct of_device_id simple_mfd_i2c_of_match[] = {
+	{ .compatible = "kontron,sl28cpld-r1" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
-- 
2.20.1


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

* [PATCH v6 04/13] irqchip: add sl28cpld interrupt controller support
  2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
                   ` (2 preceding siblings ...)
  2020-07-25 23:18 ` [PATCH v6 03/13] mfd: simple-mfd-i2c: add sl28cpld support Michael Walle
@ 2020-07-25 23:18 ` Michael Walle
  2020-07-25 23:18 ` [PATCH v6 05/13] watchdog: add support for sl28cpld watchdog Michael Walle
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle

Add support for the interrupt controller inside the sl28 CPLD management
controller.

The interrupt controller can handle at most 8 interrupts and is really
simplistic and consists only of an interrupt mask and an interrupt
pending register.

Signed-off-by: Michael Walle <michael@walle.cc>
Acked-by: Marc Zyngier <maz@kernel.org>
---
Changes since v5:
 - none

Changes since v4:
 - update copyright year
 - don't use "int irq" instead of "unsigne int irq", because
   platform_get_irq() might return a negative error code. Found by "kernel
   test robot <lkp@intel.com>
 - remove comma in terminator line of the compatible strings list,
   suggested by Andy
 - use newer devm_regmap_add_irq_chip_fwnode()
 - don't use KBUID_MODNAME, suggested by Andy
 - remove the platform device table

Changes since v3:
 - see cover letter

 drivers/irqchip/Kconfig        |  8 +++
 drivers/irqchip/Makefile       |  1 +
 drivers/irqchip/irq-sl28cpld.c | 96 ++++++++++++++++++++++++++++++++++
 3 files changed, 105 insertions(+)
 create mode 100644 drivers/irqchip/irq-sl28cpld.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 216b3b8392b5..b23d23eed3a5 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -246,6 +246,14 @@ config RENESAS_RZA1_IRQC
 	  Enable support for the Renesas RZ/A1 Interrupt Controller, to use up
 	  to 8 external interrupts with configurable sense select.
 
+config SL28CPLD_INTC
+	bool "Kontron sl28cpld IRQ controller"
+	select MFD_SIMPLE_MFD_I2C
+	select REGMAP_IRQ
+	help
+	  Interrupt controller driver for the board management controller
+	  found on the Kontron sl28 CPLD.
+
 config ST_IRQCHIP
 	bool
 	select REGMAP
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 133f9c45744a..39af7d89204d 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -111,3 +111,4 @@ obj-$(CONFIG_LOONGSON_HTPIC)		+= irq-loongson-htpic.o
 obj-$(CONFIG_LOONGSON_HTVEC)		+= irq-loongson-htvec.o
 obj-$(CONFIG_LOONGSON_PCH_PIC)		+= irq-loongson-pch-pic.o
 obj-$(CONFIG_LOONGSON_PCH_MSI)		+= irq-loongson-pch-msi.o
+obj-$(CONFIG_SL28CPLD_INTC)		+= irq-sl28cpld.o
diff --git a/drivers/irqchip/irq-sl28cpld.c b/drivers/irqchip/irq-sl28cpld.c
new file mode 100644
index 000000000000..0aa50d025ef6
--- /dev/null
+++ b/drivers/irqchip/irq-sl28cpld.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * sl28cpld interrupt controller driver
+ *
+ * Copyright 2020 Kontron Europe GmbH
+ */
+
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#define INTC_IE 0x00
+#define INTC_IP 0x01
+
+static const struct regmap_irq sl28cpld_irqs[] = {
+	REGMAP_IRQ_REG_LINE(0, 8),
+	REGMAP_IRQ_REG_LINE(1, 8),
+	REGMAP_IRQ_REG_LINE(2, 8),
+	REGMAP_IRQ_REG_LINE(3, 8),
+	REGMAP_IRQ_REG_LINE(4, 8),
+	REGMAP_IRQ_REG_LINE(5, 8),
+	REGMAP_IRQ_REG_LINE(6, 8),
+	REGMAP_IRQ_REG_LINE(7, 8),
+};
+
+struct sl28cpld_intc {
+	struct regmap *regmap;
+	struct regmap_irq_chip chip;
+	struct regmap_irq_chip_data *irq_data;
+};
+
+static int sl28cpld_intc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct sl28cpld_intc *irqchip;
+	int irq;
+	u32 base;
+	int ret;
+
+	if (!dev->parent)
+		return -ENODEV;
+
+	irqchip = devm_kzalloc(dev, sizeof(*irqchip), GFP_KERNEL);
+	if (!irqchip)
+		return -ENOMEM;
+
+	irqchip->regmap = dev_get_regmap(dev->parent, NULL);
+	if (!irqchip->regmap)
+		return -ENODEV;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	ret = device_property_read_u32(&pdev->dev, "reg", &base);
+	if (ret)
+		return -EINVAL;
+
+	irqchip->chip.name = "sl28cpld-intc";
+	irqchip->chip.irqs = sl28cpld_irqs;
+	irqchip->chip.num_irqs = ARRAY_SIZE(sl28cpld_irqs);
+	irqchip->chip.num_regs = 1;
+	irqchip->chip.status_base = base + INTC_IP;
+	irqchip->chip.mask_base = base + INTC_IE;
+	irqchip->chip.mask_invert = true,
+	irqchip->chip.ack_base = base + INTC_IP;
+
+	return devm_regmap_add_irq_chip_fwnode(dev, dev_fwnode(dev),
+					       irqchip->regmap, irq,
+					       IRQF_SHARED | IRQF_ONESHOT, 0,
+					       &irqchip->chip,
+					       &irqchip->irq_data);
+}
+
+static const struct of_device_id sl28cpld_intc_of_match[] = {
+	{ .compatible = "kontron,sl28cpld-intc" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, sl28cpld_intc_of_match);
+
+static struct platform_driver sl28cpld_intc_driver = {
+	.probe = sl28cpld_intc_probe,
+	.driver = {
+		.name = "sl28cpld-intc",
+		.of_match_table = sl28cpld_intc_of_match,
+	}
+};
+module_platform_driver(sl28cpld_intc_driver);
+
+MODULE_DESCRIPTION("sl28cpld Interrupt Controller Driver");
+MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
+MODULE_LICENSE("GPL");
-- 
2.20.1


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

* [PATCH v6 05/13] watchdog: add support for sl28cpld watchdog
  2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
                   ` (3 preceding siblings ...)
  2020-07-25 23:18 ` [PATCH v6 04/13] irqchip: add sl28cpld interrupt controller support Michael Walle
@ 2020-07-25 23:18 ` Michael Walle
  2020-07-25 23:18 ` [PATCH v6 06/13] pwm: add support for sl28cpld PWM controller Michael Walle
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle

Add support for the watchdog of the sl28cpld board management
controller. This is part of a multi-function device driver.

Signed-off-by: Michael Walle <michael@walle.cc>
Acked-by: Guenter Roeck <linux@roeck-us.net>
---
Changes since v5:
 - none

Changes since v4:
 - update copyright year
 - remove #include <linux/of_device.h>, suggested by Andy.
 - slightly reworked the error code handling (only style), suggested by
   Andy.
 - Don't use "if (ret < 0)", but only "if (ret)", suggested by Andy.
 - remove comma in terminator line of the compatible strings list
 - don't use KBUID_MODNAME
 - remove the platform device table

Changes since v3:
 - see cover letter

 drivers/watchdog/Kconfig        |  11 ++
 drivers/watchdog/Makefile       |   1 +
 drivers/watchdog/sl28cpld_wdt.c | 229 ++++++++++++++++++++++++++++++++
 3 files changed, 241 insertions(+)
 create mode 100644 drivers/watchdog/sl28cpld_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 4f4687c46d38..c4115ea0d3f8 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -340,6 +340,17 @@ config MLX_WDT
 	  To compile this driver as a module, choose M here: the
 	  module will be called mlx-wdt.
 
+config SL28CPLD_WATCHDOG
+	tristate "Kontron sl28cpld Watchdog"
+	select MFD_SIMPLE_MFD_I2C
+	select WATCHDOG_CORE
+	help
+	  Say Y here to include support for the watchdog timer
+	  on the Kontron sl28 CPLD.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called sl28cpld_wdt.
+
 # ALPHA Architecture
 
 # ARM Architecture
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 97bed1d3d97c..aa6e41126901 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -225,3 +225,4 @@ obj-$(CONFIG_MENF21BMC_WATCHDOG) += menf21bmc_wdt.o
 obj-$(CONFIG_MENZ069_WATCHDOG) += menz69_wdt.o
 obj-$(CONFIG_RAVE_SP_WATCHDOG) += rave-sp-wdt.o
 obj-$(CONFIG_STPMIC1_WATCHDOG) += stpmic1_wdt.o
+obj-$(CONFIG_SL28CPLD_WATCHDOG) += sl28cpld_wdt.o
diff --git a/drivers/watchdog/sl28cpld_wdt.c b/drivers/watchdog/sl28cpld_wdt.c
new file mode 100644
index 000000000000..a45047d8d9ab
--- /dev/null
+++ b/drivers/watchdog/sl28cpld_wdt.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * sl28cpld watchdog driver
+ *
+ * Copyright 2020 Kontron Europe GmbH
+ */
+
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/watchdog.h>
+
+/*
+ * Watchdog timer block registers.
+ */
+#define WDT_CTRL			0x00
+#define  WDT_CTRL_EN			BIT(0)
+#define  WDT_CTRL_LOCK			BIT(2)
+#define  WDT_CTRL_ASSERT_SYS_RESET	BIT(6)
+#define  WDT_CTRL_ASSERT_WDT_TIMEOUT	BIT(7)
+#define WDT_TIMEOUT			0x01
+#define WDT_KICK			0x02
+#define  WDT_KICK_VALUE			0x6b
+#define WDT_COUNT			0x03
+
+#define WDT_DEFAULT_TIMEOUT		10
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+static int timeout;
+module_param(timeout, int, 0);
+MODULE_PARM_DESC(timeout, "Initial watchdog timeout in seconds");
+
+struct sl28cpld_wdt {
+	struct watchdog_device wdd;
+	struct regmap *regmap;
+	u32 offset;
+	bool assert_wdt_timeout;
+};
+
+static int sl28cpld_wdt_ping(struct watchdog_device *wdd)
+{
+	struct sl28cpld_wdt *wdt = watchdog_get_drvdata(wdd);
+
+	return regmap_write(wdt->regmap, wdt->offset + WDT_KICK,
+			    WDT_KICK_VALUE);
+}
+
+static int sl28cpld_wdt_start(struct watchdog_device *wdd)
+{
+	struct sl28cpld_wdt *wdt = watchdog_get_drvdata(wdd);
+	unsigned int val;
+
+	val = WDT_CTRL_EN | WDT_CTRL_ASSERT_SYS_RESET;
+	if (wdt->assert_wdt_timeout)
+		val |= WDT_CTRL_ASSERT_WDT_TIMEOUT;
+	if (nowayout)
+		val |= WDT_CTRL_LOCK;
+
+	return regmap_update_bits(wdt->regmap, wdt->offset + WDT_CTRL,
+				  val, val);
+}
+
+static int sl28cpld_wdt_stop(struct watchdog_device *wdd)
+{
+	struct sl28cpld_wdt *wdt = watchdog_get_drvdata(wdd);
+
+	return regmap_update_bits(wdt->regmap, wdt->offset + WDT_CTRL,
+				  WDT_CTRL_EN, 0);
+}
+
+static unsigned int sl28cpld_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+	struct sl28cpld_wdt *wdt = watchdog_get_drvdata(wdd);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(wdt->regmap, wdt->offset + WDT_COUNT, &val);
+	if (ret)
+		return 0;
+
+	return val;
+}
+
+static int sl28cpld_wdt_set_timeout(struct watchdog_device *wdd,
+				    unsigned int timeout)
+{
+	struct sl28cpld_wdt *wdt = watchdog_get_drvdata(wdd);
+	int ret;
+
+	ret = regmap_write(wdt->regmap, wdt->offset + WDT_TIMEOUT, timeout);
+	if (ret)
+		return ret;
+
+	wdd->timeout = timeout;
+
+	return 0;
+}
+
+static const struct watchdog_info sl28cpld_wdt_info = {
+	.options = WDIOF_MAGICCLOSE | WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
+	.identity = "sl28cpld watchdog",
+};
+
+static struct watchdog_ops sl28cpld_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = sl28cpld_wdt_start,
+	.stop = sl28cpld_wdt_stop,
+	.ping = sl28cpld_wdt_ping,
+	.set_timeout = sl28cpld_wdt_set_timeout,
+	.get_timeleft = sl28cpld_wdt_get_timeleft,
+};
+
+static int sl28cpld_wdt_probe(struct platform_device *pdev)
+{
+	struct watchdog_device *wdd;
+	struct sl28cpld_wdt *wdt;
+	unsigned int status;
+	unsigned int val;
+	int ret;
+
+	if (!pdev->dev.parent)
+		return -ENODEV;
+
+	wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
+	if (!wdt)
+		return -ENOMEM;
+
+	wdt->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!wdt->regmap)
+		return -ENODEV;
+
+	ret = device_property_read_u32(&pdev->dev, "reg", &wdt->offset);
+	if (ret)
+		return -EINVAL;
+
+	wdt->assert_wdt_timeout = device_property_read_bool(&pdev->dev,
+							    "kontron,assert-wdt-timeout-pin");
+
+	/* initialize struct watchdog_device */
+	wdd = &wdt->wdd;
+	wdd->parent = &pdev->dev;
+	wdd->info = &sl28cpld_wdt_info;
+	wdd->ops = &sl28cpld_wdt_ops;
+	wdd->min_timeout = 1;
+	wdd->max_timeout = 255;
+
+	watchdog_set_drvdata(wdd, wdt);
+	watchdog_stop_on_reboot(wdd);
+
+	/*
+	 * Read the status early, in case of an error, we haven't modified the
+	 * hardware.
+	 */
+	ret = regmap_read(wdt->regmap, wdt->offset + WDT_CTRL, &status);
+	if (ret)
+		return ret;
+
+	/*
+	 * Initial timeout value, may be overwritten by device tree or module
+	 * parmeter in watchdog_init_timeout().
+	 *
+	 * Reading a zero here means that either the hardware has a default
+	 * value of zero (which is very unlikely and definitely a hardware
+	 * bug) or the bootloader set it to zero. In any case, we handle
+	 * this case gracefully and set out own timeout.
+	 */
+	ret = regmap_read(wdt->regmap, wdt->offset + WDT_TIMEOUT, &val);
+	if (ret)
+		return ret;
+
+	if (val)
+		wdd->timeout = val;
+	else
+		wdd->timeout = WDT_DEFAULT_TIMEOUT;
+
+	watchdog_init_timeout(wdd, timeout, &pdev->dev);
+	sl28cpld_wdt_set_timeout(wdd, wdd->timeout);
+
+	/* if the watchdog is locked, we set nowayout */
+	if (status & WDT_CTRL_LOCK)
+		nowayout = true;
+	watchdog_set_nowayout(wdd, nowayout);
+
+	/*
+	 * If watchdog is already running, keep it enabled, but make
+	 * sure its mode is set correctly.
+	 */
+	if (status & WDT_CTRL_EN) {
+		sl28cpld_wdt_start(wdd);
+		set_bit(WDOG_HW_RUNNING, &wdd->status);
+	}
+
+	ret = devm_watchdog_register_device(&pdev->dev, wdd);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to register watchdog device\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "initial timeout %d sec%s\n",
+		 wdd->timeout, nowayout ? ", nowayout" : "");
+
+	return 0;
+}
+
+static const struct of_device_id sl28cpld_wdt_of_match[] = {
+	{ .compatible = "kontron,sl28cpld-wdt" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, sl28cpld_wdt_of_match);
+
+static struct platform_driver sl28cpld_wdt_driver = {
+	.probe = sl28cpld_wdt_probe,
+	.driver = {
+		.name = "sl28cpld-wdt",
+		.of_match_table = sl28cpld_wdt_of_match,
+	},
+};
+module_platform_driver(sl28cpld_wdt_driver);
+
+MODULE_DESCRIPTION("sl28cpld Watchdog Driver");
+MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
+MODULE_LICENSE("GPL");
-- 
2.20.1


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

* [PATCH v6 06/13] pwm: add support for sl28cpld PWM controller
  2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
                   ` (4 preceding siblings ...)
  2020-07-25 23:18 ` [PATCH v6 05/13] watchdog: add support for sl28cpld watchdog Michael Walle
@ 2020-07-25 23:18 ` Michael Walle
  2020-07-27  7:30   ` Thierry Reding
  2020-07-28  7:43   ` Uwe Kleine-König
  2020-07-25 23:18 ` [PATCH v6 07/13] gpio: add support for the sl28cpld GPIO controller Michael Walle
                   ` (6 subsequent siblings)
  12 siblings, 2 replies; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle

Add support for the PWM controller of the sl28cpld board management
controller. This is part of a multi-function device driver.

The controller has one PWM channel and can just generate four distinct
frequencies.

Signed-off-by: Michael Walle <michael@walle.cc>
---
Changes since v5:
 - added brief description of the PWM hardware implementation
 - added hardware limitations
 - dropped the frequency mode table, instead calculate the prescaler
   value on the fly.
 - round the requested parameters instead of support just distinct
   periods.
 - prefix the macros by SL28CPLD_ to make them less generic
 - set polarity to PWM_POLARITY_NORMAL and reject inverted polarity
   requests.
 - apply the workaround just for prescaler value of 0.
 - make errors during probing more verbose

Changes since v4:
 - update copyright year
 - remove #include <linux/of_device.h>, suggested by Andy.
 - make the pwm mode table look nicer, suggested by Lee.
 - use dev_get_drvdata(chip->dev) instead of container_of(), suggested by
   Lee.
 - use whole sentence in comments, suggested by Lee.
 - renamed the local "struct sl28cpld_pwm" variable to "priv" everywhere,
   suggested by Lee.
 - use pwm_{get,set}_relative_duty_cycle(), suggested by Andy.
 - make the comment about the 250Hz hardware limitation clearer
 - don't use "if (ret < 0)", but only "if (ret)", suggested by Andy.
 - don't use KBUID_MODNAME
 - remove comma in terminator line of the compatible strings list
 - remove the platform device table

Changes since v3:
 - see cover letter

 drivers/pwm/Kconfig        |  10 ++
 drivers/pwm/Makefile       |   1 +
 drivers/pwm/pwm-sl28cpld.c | 223 +++++++++++++++++++++++++++++++++++++
 3 files changed, 234 insertions(+)
 create mode 100644 drivers/pwm/pwm-sl28cpld.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 7dbcf6973d33..a0d50d70c3b9 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -428,6 +428,16 @@ config PWM_SIFIVE
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-sifive.
 
+config PWM_SL28CPLD
+	tristate "Kontron sl28cpld PWM support"
+	select MFD_SIMPLE_MFD_I2C
+	help
+	  Generic PWM framework driver for board management controller
+	  found on the Kontron sl28 CPLD.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-sl28cpld.
+
 config PWM_SPEAR
 	tristate "STMicroelectronics SPEAr PWM support"
 	depends on PLAT_SPEAR || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 2c2ba0a03557..cbdcd55d69ee 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_PWM_RENESAS_TPU)	+= pwm-renesas-tpu.o
 obj-$(CONFIG_PWM_ROCKCHIP)	+= pwm-rockchip.o
 obj-$(CONFIG_PWM_SAMSUNG)	+= pwm-samsung.o
 obj-$(CONFIG_PWM_SIFIVE)	+= pwm-sifive.o
+obj-$(CONFIG_PWM_SL28CPLD)	+= pwm-sl28cpld.o
 obj-$(CONFIG_PWM_SPEAR)		+= pwm-spear.o
 obj-$(CONFIG_PWM_SPRD)		+= pwm-sprd.o
 obj-$(CONFIG_PWM_STI)		+= pwm-sti.o
diff --git a/drivers/pwm/pwm-sl28cpld.c b/drivers/pwm/pwm-sl28cpld.c
new file mode 100644
index 000000000000..956fa09f3aba
--- /dev/null
+++ b/drivers/pwm/pwm-sl28cpld.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * sl28cpld PWM driver
+ *
+ * Copyright (c) 2020 Michael Walle <michael@walle.cc>
+ *
+ * There is no public datasheet available for this PWM core. But it is easy
+ * enough to be briefly explained. It consists of one 8-bit counter. The PWM
+ * supports four distinct frequencies by selecting when to reset the counter.
+ * With the prescaler setting you can select which bit of the counter is used
+ * to reset it. This implies that the higher the frequency the less remaining
+ * bits are available for the actual counter.
+ *
+ * Let cnt[7:0] be the counter, clocked at 32kHz:
+ * +-----------+--------+--------------+-----------+
+ * | prescaler |  reset | counter bits | frequency |
+ * +-----------+--------+--------------+-----------+
+ * |         0 | cnt[7] |     cnt[6:0] |     250Hz |
+ * |         1 | cnt[6] |     cnt[5:0] |     500Hz |
+ * |         2 | cnt[5] |     cnt[4:0] |      1kHz |
+ * |         3 | cnt[4] |     cnt[3:0] |      2kHz |
+ * +-----------+--------+--------------+-----------+
+ *
+ * Limitations:
+ * - The hardware cannot generate a 100% duty cycle if the prescaler is 0.
+ * - The hardware cannot atomically set the prescaler and the counter value,
+ *   which might lead to glitches and inconsistent states if a write fails.
+ * - The counter is not reset if you switch the prescaler which leads
+ *   to glitches, too.
+ * - The duty cycle will switch immediately and not after a complete cycle.
+ * - Depending on the actual implementation, disabling the PWM might have
+ *   side effects. For example, if the output pin is shared with a GPIO pin
+ *   it will automatically switch back to GPIO mode.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+
+/*
+ * PWM timer block registers.
+ */
+#define SL28CPLD_PWM_CTRL			0x00
+#define   SL28CPLD_PWM_CTRL_ENABLE		BIT(7)
+#define   SL28CPLD_PWM_CTRL_PRESCALER_MASK	GENMASK(1, 0)
+#define SL28CPLD_PWM_CYCLE			0x01
+#define   SL28CPLD_PWM_CYCLE_MAX		GENMASK(6, 0)
+
+#define SL28CPLD_PWM_CLK			32000 /* 32 kHz */
+#define SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler)	(1 << (7 - (prescaler)))
+#define SL28CPLD_PWM_PERIOD(prescaler) \
+	(NSEC_PER_SEC / SL28CPLD_PWM_CLK * SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler))
+
+/*
+ * We calculate the duty cycle like this:
+ *   duty_cycle_ns = pwm_cycle_reg * max_period_ns / max_duty_cycle
+ *
+ * With
+ *   max_period_ns = (1 << 7 - prescaler) / pwm_clk * NSEC_PER_SEC
+ *   max_duty_cycle = 1 << (7 - prescaler)
+ * this then simplifies to:
+ *   duty_cycle_ns = pwm_cycle_reg / pwm_clk * NSEC_PER_SEC
+ */
+#define SL28CPLD_PWM_TO_DUTY_CYCLE(reg) \
+	(NSEC_PER_SEC / SL28CPLD_PWM_CLK * (reg))
+#define SL28CPLD_PWM_FROM_DUTY_CYCLE(duty_cycle) \
+	(DIV_ROUND_DOWN_ULL((duty_cycle), NSEC_PER_SEC / SL28CPLD_PWM_CLK))
+
+struct sl28cpld_pwm {
+	struct pwm_chip pwm_chip;
+	struct regmap *regmap;
+	u32 offset;
+};
+
+static void sl28cpld_pwm_get_state(struct pwm_chip *chip,
+				   struct pwm_device *pwm,
+				   struct pwm_state *state)
+{
+	struct sl28cpld_pwm *priv = dev_get_drvdata(chip->dev);
+	unsigned int reg;
+	int prescaler;
+
+	regmap_read(priv->regmap, priv->offset + SL28CPLD_PWM_CTRL, &reg);
+
+	state->enabled = reg & SL28CPLD_PWM_CTRL_ENABLE;
+
+	prescaler = FIELD_GET(SL28CPLD_PWM_CTRL_PRESCALER_MASK, reg);
+	state->period = SL28CPLD_PWM_PERIOD(prescaler);
+
+	regmap_read(priv->regmap, priv->offset + SL28CPLD_PWM_CYCLE, &reg);
+	state->duty_cycle = SL28CPLD_PWM_TO_DUTY_CYCLE(reg);
+	state->polarity = PWM_POLARITY_NORMAL;
+}
+
+static int sl28cpld_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			      const struct pwm_state *state)
+{
+	struct sl28cpld_pwm *priv = dev_get_drvdata(chip->dev);
+	unsigned int cycle, prescaler;
+	int ret;
+	u8 ctrl;
+
+	/* Polarity inversion is not supported */
+	if (state->polarity != PWM_POLARITY_NORMAL)
+		return -EINVAL;
+
+	/*
+	 * Calculate the prescaler. Pick the the biggest period that isn't
+	 * bigger than the requested period.
+	 */
+	prescaler = DIV_ROUND_UP_ULL(SL28CPLD_PWM_PERIOD(0), state->period);
+	prescaler = order_base_2(prescaler);
+
+	if (prescaler > field_max(SL28CPLD_PWM_CTRL_PRESCALER_MASK))
+		return -ERANGE;
+
+	ctrl = FIELD_PREP(SL28CPLD_PWM_CTRL_PRESCALER_MASK, prescaler);
+	if (state->enabled)
+		ctrl |= SL28CPLD_PWM_CTRL_ENABLE;
+
+	cycle = SL28CPLD_PWM_FROM_DUTY_CYCLE(state->duty_cycle);
+	cycle = min_t(unsigned int, cycle, SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler));
+
+	/*
+	 * Work around the hardware limitation. See also above. Trap 100% duty
+	 * cycle if the prescaler is 0. Set prescaler to 1 instead. We don't
+	 * care about the frequency because its "all-one" in either case.
+	 *
+	 * We don't need to check the actual prescaler setting, because only
+	 * if the prescaler is 0 we can have this particular value.
+	 */
+	if (cycle == SL28CPLD_PWM_MAX_DUTY_CYCLE(0)) {
+		ctrl &= ~SL28CPLD_PWM_CTRL_PRESCALER_MASK;
+		ctrl |= FIELD_PREP(SL28CPLD_PWM_CTRL_PRESCALER_MASK, 1);
+		cycle = SL28CPLD_PWM_MAX_DUTY_CYCLE(1);
+	}
+
+	ret = regmap_write(priv->regmap, priv->offset + SL28CPLD_PWM_CTRL, ctrl);
+	if (ret)
+		return ret;
+
+	return regmap_write(priv->regmap, priv->offset + SL28CPLD_PWM_CYCLE, (u8)cycle);
+}
+
+static const struct pwm_ops sl28cpld_pwm_ops = {
+	.apply = sl28cpld_pwm_apply,
+	.get_state = sl28cpld_pwm_get_state,
+	.owner = THIS_MODULE,
+};
+
+static int sl28cpld_pwm_probe(struct platform_device *pdev)
+{
+	struct sl28cpld_pwm *priv;
+	struct pwm_chip *chip;
+	int ret;
+
+	if (!pdev->dev.parent)
+		return -ENODEV;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!priv->regmap)
+		return -ENODEV;
+
+	ret = device_property_read_u32(&pdev->dev, "reg", &priv->offset);
+	if (ret) {
+		dev_err(&pdev->dev, "no 'reg' property found (%pe)\n",
+			ERR_PTR(ret));
+		return -EINVAL;
+	}
+
+	/* Initialize the pwm_chip structure */
+	chip = &priv->pwm_chip;
+	chip->dev = &pdev->dev;
+	chip->ops = &sl28cpld_pwm_ops;
+	chip->base = -1;
+	chip->npwm = 1;
+
+	ret = pwmchip_add(&priv->pwm_chip);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add PWM chip (%pe)",
+			ERR_PTR(ret));
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+}
+
+static int sl28cpld_pwm_remove(struct platform_device *pdev)
+{
+	struct sl28cpld_pwm *priv = platform_get_drvdata(pdev);
+
+	return pwmchip_remove(&priv->pwm_chip);
+}
+
+static const struct of_device_id sl28cpld_pwm_of_match[] = {
+	{ .compatible = "kontron,sl28cpld-pwm" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, sl28cpld_pwm_of_match);
+
+static struct platform_driver sl28cpld_pwm_driver = {
+	.probe = sl28cpld_pwm_probe,
+	.remove	= sl28cpld_pwm_remove,
+	.driver = {
+		.name = "sl28cpld-pwm",
+		.of_match_table = sl28cpld_pwm_of_match,
+	},
+};
+module_platform_driver(sl28cpld_pwm_driver);
+
+MODULE_DESCRIPTION("sl28cpld PWM Driver");
+MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
+MODULE_LICENSE("GPL");
-- 
2.20.1


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

* [PATCH v6 07/13] gpio: add support for the sl28cpld GPIO controller
  2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
                   ` (5 preceding siblings ...)
  2020-07-25 23:18 ` [PATCH v6 06/13] pwm: add support for sl28cpld PWM controller Michael Walle
@ 2020-07-25 23:18 ` Michael Walle
  2020-07-26  9:16   ` Andy Shevchenko
  2020-07-25 23:18 ` [PATCH v6 08/13] hwmon: add support for the sl28cpld hardware monitoring controller Michael Walle
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle

Add support for the GPIO controller of the sl28 board management
controller. This driver is part of a multi-function device.

A controller has 8 lines. There are three different flavors:
full-featured GPIO with interrupt support, input-only and output-only.

Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
Changes since v5:
 - added "select REGMAP_IRQ"

Changes since v4:
 - update copyright year
 - remove #include <linux/of_device.h>, suggested by Andy.
 - use device_get_match_data(), suggested by Andy.
 - drop the irq_support variable, instead call _init_irq() directly,
   suggested by Andy.
 - also move the irq code a bit around to make it look nicer
 - drop "struct sl28cpld_gpio". We don't need to actually store the
   irq_chip and irq_chip_data, everything is device resource managed.
 - use 100 chars line limit, suggested by Andy.
 - remove the platform device table
 - don't use KBUID_MODNAME

Changes since v3:
 - see cover letter

 drivers/gpio/Kconfig         |  12 +++
 drivers/gpio/Makefile        |   1 +
 drivers/gpio/gpio-sl28cpld.c | 161 +++++++++++++++++++++++++++++++++++
 3 files changed, 174 insertions(+)
 create mode 100644 drivers/gpio/gpio-sl28cpld.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 8030fd91a3cc..f6a547b76432 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1223,6 +1223,18 @@ config GPIO_RC5T583
 	  This driver provides the support for driving/reading the gpio pins
 	  of RC5T583 device through standard gpio library.
 
+config GPIO_SL28CPLD
+	tristate "Kontron sl28cpld GPIO support"
+	select MFD_SIMPLE_MFD_I2C
+	select GPIO_REGMAP
+	select GPIOLIB_IRQCHIP
+	select REGMAP_IRQ
+	help
+	  This enables support for the GPIOs found on the Kontron sl28 CPLD.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called gpio-sl28cpld.
+
 config GPIO_STMPE
 	bool "STMPE GPIOs"
 	depends on MFD_STMPE
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4f9abff4f2dc..c3a4e7c94a91 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -132,6 +132,7 @@ obj-$(CONFIG_GPIO_SCH311X)		+= gpio-sch311x.o
 obj-$(CONFIG_GPIO_SCH)			+= gpio-sch.o
 obj-$(CONFIG_GPIO_SIFIVE)		+= gpio-sifive.o
 obj-$(CONFIG_GPIO_SIOX)			+= gpio-siox.o
+obj-$(CONFIG_GPIO_SL28CPLD)		+= gpio-sl28cpld.o
 obj-$(CONFIG_GPIO_SODAVILLE)		+= gpio-sodaville.o
 obj-$(CONFIG_GPIO_SPEAR_SPICS)		+= gpio-spear-spics.o
 obj-$(CONFIG_GPIO_SPRD)			+= gpio-sprd.o
diff --git a/drivers/gpio/gpio-sl28cpld.c b/drivers/gpio/gpio-sl28cpld.c
new file mode 100644
index 000000000000..889b8f5622c2
--- /dev/null
+++ b/drivers/gpio/gpio-sl28cpld.c
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * sl28cpld GPIO driver
+ *
+ * Copyright 2020 Michael Walle <michael@walle.cc>
+ */
+
+#include <linux/device.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/regmap.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/* GPIO flavor */
+#define GPIO_REG_DIR	0x00
+#define GPIO_REG_OUT	0x01
+#define GPIO_REG_IN	0x02
+#define GPIO_REG_IE	0x03
+#define GPIO_REG_IP	0x04
+
+/* input-only flavor */
+#define GPI_REG_IN	0x00
+
+/* output-only flavor */
+#define GPO_REG_OUT	0x00
+
+enum sl28cpld_gpio_type {
+	SL28CPLD_GPIO = 1,
+	SL28CPLD_GPI,
+	SL28CPLD_GPO,
+};
+
+static const struct regmap_irq sl28cpld_gpio_irqs[] = {
+	REGMAP_IRQ_REG_LINE(0, 8),
+	REGMAP_IRQ_REG_LINE(1, 8),
+	REGMAP_IRQ_REG_LINE(2, 8),
+	REGMAP_IRQ_REG_LINE(3, 8),
+	REGMAP_IRQ_REG_LINE(4, 8),
+	REGMAP_IRQ_REG_LINE(5, 8),
+	REGMAP_IRQ_REG_LINE(6, 8),
+	REGMAP_IRQ_REG_LINE(7, 8),
+};
+
+static int sl28cpld_gpio_irq_init(struct platform_device *pdev,
+				  unsigned int base,
+				  struct gpio_regmap_config *config)
+{
+	struct regmap_irq_chip_data *irq_data;
+	struct regmap_irq_chip *irq_chip;
+	struct device *dev = &pdev->dev;
+	int irq, ret;
+
+	if (!device_property_read_bool(dev, "interrupt-controller"))
+		return 0;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	irq_chip = devm_kzalloc(dev, sizeof(*irq_chip), GFP_KERNEL);
+	if (!irq_chip)
+		return -ENOMEM;
+
+	irq_chip->name = "sl28cpld-gpio-irq",
+	irq_chip->irqs = sl28cpld_gpio_irqs;
+	irq_chip->num_irqs = ARRAY_SIZE(sl28cpld_gpio_irqs);
+	irq_chip->num_regs = 1;
+	irq_chip->status_base = base + GPIO_REG_IP;
+	irq_chip->mask_base = base + GPIO_REG_IE;
+	irq_chip->mask_invert = true,
+	irq_chip->ack_base = base + GPIO_REG_IP;
+
+	ret = devm_regmap_add_irq_chip_fwnode(dev, dev_fwnode(dev),
+					      config->regmap, irq,
+					      IRQF_SHARED | IRQF_ONESHOT,
+					      0, irq_chip, &irq_data);
+	if (ret)
+		return ret;
+
+	config->irq_domain = regmap_irq_get_domain(irq_data);
+
+	return 0;
+}
+
+static int sl28cpld_gpio_probe(struct platform_device *pdev)
+{
+	struct gpio_regmap_config config = {0};
+	enum sl28cpld_gpio_type type;
+	struct regmap *regmap;
+	u32 base;
+	int ret;
+
+	if (!pdev->dev.parent)
+		return -ENODEV;
+
+	type = (uintptr_t)device_get_match_data(&pdev->dev);
+	if (!type)
+		return -ENODEV;
+
+	ret = device_property_read_u32(&pdev->dev, "reg", &base);
+	if (ret)
+		return -EINVAL;
+
+	regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!regmap)
+		return -ENODEV;
+
+	config.regmap = regmap;
+	config.parent = &pdev->dev;
+	config.ngpio = 8;
+
+	switch (type) {
+	case SL28CPLD_GPIO:
+		config.reg_dat_base = base + GPIO_REG_IN;
+		config.reg_set_base = base + GPIO_REG_OUT;
+		/* reg_dir_out_base might be zero */
+		config.reg_dir_out_base = GPIO_REGMAP_ADDR(base + GPIO_REG_DIR);
+
+		/* This type supports interrupts */
+		ret = sl28cpld_gpio_irq_init(pdev, base, &config);
+		if (ret)
+			return ret;
+		break;
+	case SL28CPLD_GPO:
+		config.reg_set_base = base + GPO_REG_OUT;
+		break;
+	case SL28CPLD_GPI:
+		config.reg_dat_base = base + GPI_REG_IN;
+		break;
+	default:
+		dev_err(&pdev->dev, "unknown type %d\n", type);
+		return -ENODEV;
+	}
+
+	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(&pdev->dev, &config));
+}
+
+static const struct of_device_id sl28cpld_gpio_of_match[] = {
+	{ .compatible = "kontron,sl28cpld-gpio", .data = (void *)SL28CPLD_GPIO },
+	{ .compatible = "kontron,sl28cpld-gpi", .data = (void *)SL28CPLD_GPI },
+	{ .compatible = "kontron,sl28cpld-gpo", .data = (void *)SL28CPLD_GPO },
+	{}
+};
+MODULE_DEVICE_TABLE(of, sl28cpld_gpio_of_match);
+
+static struct platform_driver sl28cpld_gpio_driver = {
+	.probe = sl28cpld_gpio_probe,
+	.driver = {
+		.name = "sl28cpld-gpio",
+		.of_match_table = sl28cpld_gpio_of_match,
+	},
+};
+module_platform_driver(sl28cpld_gpio_driver);
+
+MODULE_DESCRIPTION("sl28cpld GPIO Driver");
+MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
+MODULE_LICENSE("GPL");
-- 
2.20.1


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

* [PATCH v6 08/13] hwmon: add support for the sl28cpld hardware monitoring controller
  2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
                   ` (6 preceding siblings ...)
  2020-07-25 23:18 ` [PATCH v6 07/13] gpio: add support for the sl28cpld GPIO controller Michael Walle
@ 2020-07-25 23:18 ` Michael Walle
  2020-07-26  9:18   ` Andy Shevchenko
  2020-07-25 23:18 ` [PATCH v6 09/13] arm64: dts: freescale: sl28: enable sl28cpld Michael Walle
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle

Add support for the hardware monitoring controller of the sl28cpld board
management controller. This driver is part of a multi-function device.

Signed-off-by: Michael Walle <michael@walle.cc>
Acked-by: Guenter Roeck <linux@roeck-us.net>
---
Changes since v5:
 - none

Changes since v4:
 - update copyright year
 - remove #include <linux/of_device.h>, suggested by Andy.
 - use PTR_ERR_OR_ZERO(), suggested by Andy.
 - remove the platform device table
 - don't use KBUID_MODNAME

Changes since v3:
 - see cover letter

 Documentation/hwmon/index.rst    |   1 +
 Documentation/hwmon/sl28cpld.rst |  36 ++++++++
 drivers/hwmon/Kconfig            |  10 +++
 drivers/hwmon/Makefile           |   1 +
 drivers/hwmon/sl28cpld-hwmon.c   | 142 +++++++++++++++++++++++++++++++
 5 files changed, 190 insertions(+)
 create mode 100644 Documentation/hwmon/sl28cpld.rst
 create mode 100644 drivers/hwmon/sl28cpld-hwmon.c

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 750d3a975d82..d90c43c82936 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -154,6 +154,7 @@ Hardware Monitoring Kernel Drivers
    sht3x
    shtc1
    sis5595
+   sl28cpld
    smm665
    smsc47b397
    smsc47m192
diff --git a/Documentation/hwmon/sl28cpld.rst b/Documentation/hwmon/sl28cpld.rst
new file mode 100644
index 000000000000..7ed65f78250c
--- /dev/null
+++ b/Documentation/hwmon/sl28cpld.rst
@@ -0,0 +1,36 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Kernel driver sl28cpld
+======================
+
+Supported chips:
+
+   * Kontron sl28cpld
+
+     Prefix: 'sl28cpld'
+
+     Datasheet: not available
+
+Authors: Michael Walle <michael@walle.cc>
+
+Description
+-----------
+
+The sl28cpld is a board management controller which also exposes a hardware
+monitoring controller. At the moment this controller supports a single fan
+supervisor. In the future there might be other flavours and additional
+hardware monitoring might be supported.
+
+The fan supervisor has a 7 bit counter register and a counter period of 1
+second. If the 7 bit counter overflows, the supervisor will automatically
+switch to x8 mode to support a wider input range at the loss of
+granularity.
+
+Sysfs entries
+-------------
+
+The following attributes are supported.
+
+======================= ========================================================
+fan1_input		Fan RPM. Assuming 2 pulses per revolution.
+======================= ========================================================
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 8dc28b26916e..a02829ec7386 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1479,6 +1479,16 @@ config SENSORS_RASPBERRYPI_HWMON
 	  This driver can also be built as a module. If so, the module
 	  will be called raspberrypi-hwmon.
 
+config SENSORS_SL28CPLD
+	tristate "Kontron sl28cpld hardware monitoring driver"
+	select MFD_SIMPLE_MFD_I2C
+	help
+	  If you say yes here you get support for the fan supervisor of the
+	  sl28cpld board management controller.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called sl28cpld-hwmon.
+
 config SENSORS_SHT15
 	tristate "Sensiron humidity and temperature sensors. SHT15 and compat."
 	depends on GPIOLIB || COMPILE_TEST
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index a8f4b35b136b..dee8511f9348 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -159,6 +159,7 @@ obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
 obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
 obj-$(CONFIG_SENSORS_SCH5627)	+= sch5627.o
 obj-$(CONFIG_SENSORS_SCH5636)	+= sch5636.o
+obj-$(CONFIG_SENSORS_SL28CPLD)	+= sl28cpld-hwmon.o
 obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
 obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
 obj-$(CONFIG_SENSORS_SHT3x)	+= sht3x.o
diff --git a/drivers/hwmon/sl28cpld-hwmon.c b/drivers/hwmon/sl28cpld-hwmon.c
new file mode 100644
index 000000000000..e48f58ec5b9c
--- /dev/null
+++ b/drivers/hwmon/sl28cpld-hwmon.c
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * sl28cpld hardware monitoring driver
+ *
+ * Copyright 2020 Kontron Europe GmbH
+ */
+
+#include <linux/bitfield.h>
+#include <linux/hwmon.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#define FAN_INPUT		0x00
+#define   FAN_SCALE_X8		BIT(7)
+#define   FAN_VALUE_MASK	GENMASK(6, 0)
+
+struct sl28cpld_hwmon {
+	struct regmap *regmap;
+	u32 offset;
+};
+
+static umode_t sl28cpld_hwmon_is_visible(const void *data,
+					 enum hwmon_sensor_types type,
+					 u32 attr, int channel)
+{
+	return 0444;
+}
+
+static int sl28cpld_hwmon_read(struct device *dev,
+			       enum hwmon_sensor_types type, u32 attr,
+			       int channel, long *input)
+{
+	struct sl28cpld_hwmon *hwmon = dev_get_drvdata(dev);
+	unsigned int value;
+	int ret;
+
+	switch (attr) {
+	case hwmon_fan_input:
+		ret = regmap_read(hwmon->regmap, hwmon->offset + FAN_INPUT,
+				  &value);
+		if (ret)
+			return ret;
+		/*
+		 * The register has a 7 bit value and 1 bit which indicates the
+		 * scale. If the MSB is set, then the lower 7 bit has to be
+		 * multiplied by 8, to get the correct reading.
+		 */
+		if (value & FAN_SCALE_X8)
+			value = FIELD_GET(FAN_VALUE_MASK, value) << 3;
+
+		/*
+		 * The counter period is 1000ms and the sysfs specification
+		 * says we should asssume 2 pulses per revolution.
+		 */
+		value *= 60 / 2;
+
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	*input = value;
+	return 0;
+}
+
+static const u32 sl28cpld_hwmon_fan_config[] = {
+	HWMON_F_INPUT,
+	0
+};
+
+static const struct hwmon_channel_info sl28cpld_hwmon_fan = {
+	.type = hwmon_fan,
+	.config = sl28cpld_hwmon_fan_config,
+};
+
+static const struct hwmon_channel_info *sl28cpld_hwmon_info[] = {
+	&sl28cpld_hwmon_fan,
+	NULL
+};
+
+static const struct hwmon_ops sl28cpld_hwmon_ops = {
+	.is_visible = sl28cpld_hwmon_is_visible,
+	.read = sl28cpld_hwmon_read,
+};
+
+static const struct hwmon_chip_info sl28cpld_hwmon_chip_info = {
+	.ops = &sl28cpld_hwmon_ops,
+	.info = sl28cpld_hwmon_info,
+};
+
+static int sl28cpld_hwmon_probe(struct platform_device *pdev)
+{
+	struct sl28cpld_hwmon *hwmon;
+	struct device *hwmon_dev;
+	int ret;
+
+	if (!pdev->dev.parent)
+		return -ENODEV;
+
+	hwmon = devm_kzalloc(&pdev->dev, sizeof(*hwmon), GFP_KERNEL);
+	if (!hwmon)
+		return -ENOMEM;
+
+	hwmon->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!hwmon->regmap)
+		return -ENODEV;
+
+	ret = device_property_read_u32(&pdev->dev, "reg", &hwmon->offset);
+	if (ret)
+		return -EINVAL;
+
+	hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
+				"sl28cpld_hwmon", hwmon,
+				&sl28cpld_hwmon_chip_info, NULL);
+	if (IS_ERR(hwmon_dev))
+		dev_err(&pdev->dev, "failed to register as hwmon device");
+
+	return PTR_ERR_OR_ZERO(hwmon_dev);
+}
+
+static const struct of_device_id sl28cpld_hwmon_of_match[] = {
+	{ .compatible = "kontron,sl28cpld-fan" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, sl28cpld_hwmon_of_match);
+
+static struct platform_driver sl28cpld_hwmon_driver = {
+	.probe = sl28cpld_hwmon_probe,
+	.driver = {
+		.name = "sl28cpld-fan",
+		.of_match_table = sl28cpld_hwmon_of_match,
+	},
+};
+module_platform_driver(sl28cpld_hwmon_driver);
+
+MODULE_DESCRIPTION("sl28cpld Hardware Monitoring Driver");
+MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
+MODULE_LICENSE("GPL");
-- 
2.20.1


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

* [PATCH v6 09/13] arm64: dts: freescale: sl28: enable sl28cpld
  2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
                   ` (7 preceding siblings ...)
  2020-07-25 23:18 ` [PATCH v6 08/13] hwmon: add support for the sl28cpld hardware monitoring controller Michael Walle
@ 2020-07-25 23:18 ` Michael Walle
  2020-07-25 23:18 ` [PATCH v6 10/13] arm64: dts: freescale: sl28: map GPIOs to input events Michael Walle
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle

Add the board management controller node.

Signed-off-by: Michael Walle <michael@walle.cc>
---
Changes since v5:
 - none

Changes since v4:
 - none

Changes since v3:
 - see cover letter

 .../freescale/fsl-ls1028a-kontron-sl28.dts    | 102 ++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
index 360b3a168c10..8712fe82727b 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
@@ -8,6 +8,7 @@
 
 /dts-v1/;
 #include "fsl-ls1028a.dtsi"
+#include <dt-bindings/interrupt-controller/irq.h>
 
 / {
 	model = "Kontron SMARC-sAL28";
@@ -170,6 +171,107 @@
 		reg = <0x32>;
 	};
 
+	sl28cpld@4a {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "kontron,sl28cpld-r1";
+		reg = <0x4a>;
+
+		watchdog@4 {
+			compatible = "kontron,sl28cpld-wdt";
+			reg = <0x4>;
+			kontron,assert-wdt-timeout-pin;
+		};
+
+		hwmon@b {
+			compatible = "kontron,sl28cpld-fan";
+			reg = <0xb>;
+		};
+
+		sl28cpld_pwm0: pwm@c {
+			#pwm-cells = <2>;
+			compatible = "kontron,sl28cpld-pwm";
+			reg = <0xc>;
+		};
+
+		sl28cpld_pwm1: pwm@e {
+			#pwm-cells = <2>;
+			compatible = "kontron,sl28cpld-pwm";
+			reg = <0xe>;
+		};
+
+		sl28cpld_gpio0: gpio@10 {
+			compatible = "kontron,sl28cpld-gpio";
+			reg = <0x10>;
+			interrupts-extended = <&gpio2 6
+					       IRQ_TYPE_EDGE_FALLING>;
+
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-line-names =
+				"GPIO0_CAM0_PWR_N", "GPIO1_CAM1_PWR_N",
+				"GPIO2_CAM0_RST_N", "GPIO3_CAM1_RST_N",
+				"GPIO4_HDA_RST_N", "GPIO5_PWM_OUT",
+				"GPIO6_TACHIN", "GPIO7";
+
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+
+		sl28cpld_gpio1: gpio@15 {
+			compatible = "kontron,sl28cpld-gpio";
+			reg = <0x15>;
+			interrupts-extended = <&gpio2 6
+					       IRQ_TYPE_EDGE_FALLING>;
+
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-line-names =
+				"GPIO8", "GPIO9", "GPIO10", "GPIO11",
+				"", "", "", "";
+
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+
+		sl28cpld_gpio2: gpio@1a {
+			compatible = "kontron,sl28cpld-gpo";
+			reg = <0x1a>;
+
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-line-names =
+				"LCD0 voltage enable",
+				"LCD0 backlight enable",
+				"eMMC reset", "LVDS bridge reset",
+				"LVDS bridge power-down",
+				"SDIO power enable",
+				"", "";
+		};
+
+		sl28cpld_gpio3: gpio@1b {
+			compatible = "kontron,sl28cpld-gpi";
+			reg = <0x1b>;
+
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-line-names =
+				"Power button", "Force recovery", "Sleep",
+				"Battery low", "Lid state", "Charging",
+				"Charger present", "";
+		};
+
+		sl28cpld_intc: interrupt-controller@1c {
+			compatible = "kontron,sl28cpld-intc";
+			reg = <0x1c>;
+			interrupts-extended = <&gpio2 6
+					       IRQ_TYPE_EDGE_FALLING>;
+
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+	};
+
 	eeprom@50 {
 		compatible = "atmel,24c32";
 		reg = <0x50>;
-- 
2.20.1


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

* [PATCH v6 10/13] arm64: dts: freescale: sl28: map GPIOs to input events
  2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
                   ` (8 preceding siblings ...)
  2020-07-25 23:18 ` [PATCH v6 09/13] arm64: dts: freescale: sl28: enable sl28cpld Michael Walle
@ 2020-07-25 23:18 ` Michael Walle
  2020-07-25 23:18 ` [PATCH v6 11/13] arm64: dts: freescale: sl28: enable LED support Michael Walle
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle

Now that we have support for GPIO lines of the SMARC connector, map the
sleep, power and lid switch signals to the corresponding keys using the
gpio-keys and gpio-keys-polled drivers. The power and sleep signals have
dedicated interrupts, thus we use these ones. The lid switch is just
mapped to a GPIO input and needs polling.

Signed-off-by: Michael Walle <michael@walle.cc>
---
Changes since v5:
 - none

Changes since v4:
 - none

Changes since v3:
 - see cover letter

 .../freescale/fsl-ls1028a-kontron-sl28.dts    | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
index 8712fe82727b..c4fd99efdbba 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
@@ -9,6 +9,8 @@
 /dts-v1/;
 #include "fsl-ls1028a.dtsi"
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
 
 / {
 	model = "Kontron SMARC-sAL28";
@@ -23,6 +25,36 @@
 		spi1 = &dspi2;
 	};
 
+	buttons0 {
+		compatible = "gpio-keys";
+
+		power-button {
+			interrupts-extended = <&sl28cpld_intc
+					       4 IRQ_TYPE_EDGE_BOTH>;
+			linux,code = <KEY_POWER>;
+			label = "Power";
+		};
+
+		sleep-button {
+			interrupts-extended = <&sl28cpld_intc
+					       5 IRQ_TYPE_EDGE_BOTH>;
+			linux,code = <KEY_SLEEP>;
+			label = "Sleep";
+		};
+	};
+
+	buttons1 {
+		compatible = "gpio-keys-polled";
+		poll-interval = <200>;
+
+		lid-switch {
+			linux,input-type = <EV_SW>;
+			linux,code = <SW_LID>;
+			gpios = <&sl28cpld_gpio3 4 GPIO_ACTIVE_LOW>;
+			label = "Lid";
+		};
+	};
+
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
-- 
2.20.1


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

* [PATCH v6 11/13] arm64: dts: freescale: sl28: enable LED support
  2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
                   ` (9 preceding siblings ...)
  2020-07-25 23:18 ` [PATCH v6 10/13] arm64: dts: freescale: sl28: map GPIOs to input events Michael Walle
@ 2020-07-25 23:18 ` Michael Walle
  2020-07-25 23:18 ` [PATCH v6 12/13] arm64: dts: freescale: sl28: enable fan support Michael Walle
  2020-07-25 23:18 ` [PATCH v6 13/13] arm64: defconfig: enable the sl28cpld board management controller Michael Walle
  12 siblings, 0 replies; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle

Now that we have support for GPIO lines of the SMARC connector, enable
LED support on the KBox A-230-LS. There are two LEDs without fixed
functions, one is yellow and one is green. Unfortunately, it is just one
multi-color LED, thus while it is possible to enable both at the same
time it is hard to tell the difference between "yellow only" and "yellow
and green".

Signed-off-by: Michael Walle <michael@walle.cc>
---
Changes since v5:
 - changed the label, suggested by Pavel Machek

Changes since v4:
 - none

Changes since v3:
 - see cover letter

 .../fsl-ls1028a-kontron-kbox-a-230-ls.dts          | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts
index 4b4cc6a1573d..8e5e289ce04f 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts
@@ -16,6 +16,20 @@
 	model = "Kontron KBox A-230-LS";
 	compatible = "kontron,kbox-a-230-ls", "kontron,sl28-var4",
 		     "kontron,sl28", "fsl,ls1028a";
+
+	leds {
+		compatible = "gpio-leds";
+
+		alarm-led {
+			label = "yellow:alarm";
+			gpios = <&sl28cpld_gpio0 0 0>;
+		};
+
+		power-led {
+			label = "green:power";
+			gpios = <&sl28cpld_gpio1 3 0>;
+		};
+	};
 };
 
 &enetc_mdio_pf3 {
-- 
2.20.1


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

* [PATCH v6 12/13] arm64: dts: freescale: sl28: enable fan support
  2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
                   ` (10 preceding siblings ...)
  2020-07-25 23:18 ` [PATCH v6 11/13] arm64: dts: freescale: sl28: enable LED support Michael Walle
@ 2020-07-25 23:18 ` Michael Walle
  2020-07-25 23:18 ` [PATCH v6 13/13] arm64: defconfig: enable the sl28cpld board management controller Michael Walle
  12 siblings, 0 replies; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle

Add a pwm-fan mapped to the PWM channel 0 which is connected to the
fan connector of the carrier.

Signed-off-by: Michael Walle <michael@walle.cc>
---
Changes since v5:
 - none

Changes since v4:
 - none

Changes since v3:
 - see cover letter

 .../dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts
index 0973a6a45217..c45d7b40e374 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts
@@ -15,6 +15,15 @@
 	compatible = "kontron,sl28-var3-ads2", "kontron,sl28-var3",
 		     "kontron,sl28", "fsl,ls1028a";
 
+	pwm-fan {
+		compatible = "pwm-fan";
+		cooling-min-state = <0>;
+		cooling-max-state = <3>;
+		#cooling-cells = <2>;
+		pwms = <&sl28cpld_pwm0 0 4000000>;
+		cooling-levels = <1 128 192 255>;
+	};
+
 	sound {
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.20.1


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

* [PATCH v6 13/13] arm64: defconfig: enable the sl28cpld board management controller
  2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
                   ` (11 preceding siblings ...)
  2020-07-25 23:18 ` [PATCH v6 12/13] arm64: dts: freescale: sl28: enable fan support Michael Walle
@ 2020-07-25 23:18 ` Michael Walle
  12 siblings, 0 replies; 35+ messages in thread
From: Michael Walle @ 2020-07-25 23:18 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Catalin Marinas, Will Deacon, Pavel Machek,
	Michael Walle

Enable the kernel modules for the board management controller "sl28cpld"
which is used on the SMARC-sAL28 board.

Signed-off-by: Michael Walle <michael@walle.cc>
---
Changes since v5:
 - new patch

 arch/arm64/configs/defconfig | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index e0f33826819f..d45a33ccf7d7 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -500,6 +500,7 @@ CONFIG_GPIO_PCA953X=y
 CONFIG_GPIO_PCA953X_IRQ=y
 CONFIG_GPIO_BD9571MWV=m
 CONFIG_GPIO_MAX77620=y
+CONFIG_GPIO_SL28CPLD=m
 CONFIG_POWER_AVS=y
 CONFIG_QCOM_CPR=y
 CONFIG_ROCKCHIP_IODOMAIN=y
@@ -513,6 +514,7 @@ CONFIG_SENSORS_ARM_SCPI=y
 CONFIG_SENSORS_LM90=m
 CONFIG_SENSORS_PWM_FAN=m
 CONFIG_SENSORS_RASPBERRYPI_HWMON=m
+CONFIG_SENSORS_SL28CPLD=m
 CONFIG_SENSORS_INA2XX=m
 CONFIG_SENSORS_INA3221=m
 CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
@@ -535,6 +537,7 @@ CONFIG_QCOM_TSENS=y
 CONFIG_QCOM_SPMI_TEMP_ALARM=m
 CONFIG_UNIPHIER_THERMAL=y
 CONFIG_WATCHDOG=y
+CONFIG_SL28CPLD_WATCHDOG=m
 CONFIG_ARM_SP805_WATCHDOG=y
 CONFIG_ARM_SBSA_WATCHDOG=y
 CONFIG_ARM_SMC_WATCHDOG=y
@@ -934,8 +937,10 @@ CONFIG_PWM_MESON=m
 CONFIG_PWM_RCAR=m
 CONFIG_PWM_ROCKCHIP=y
 CONFIG_PWM_SAMSUNG=y
+CONFIG_PWM_SL28CPLD=m
 CONFIG_PWM_SUN4I=m
 CONFIG_PWM_TEGRA=m
+CONFIG_SL28CPLD_INTC=y
 CONFIG_QCOM_PDC=y
 CONFIG_RESET_QCOM_AOSS=y
 CONFIG_RESET_QCOM_PDC=m
-- 
2.20.1


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

* Re: [PATCH v6 07/13] gpio: add support for the sl28cpld GPIO controller
  2020-07-25 23:18 ` [PATCH v6 07/13] gpio: add support for the sl28cpld GPIO controller Michael Walle
@ 2020-07-26  9:16   ` Andy Shevchenko
  0 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2020-07-26  9:16 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Catalin Marinas, Will Deacon, Pavel Machek

On Sun, Jul 26, 2020 at 01:18:28AM +0200, Michael Walle wrote:
> Add support for the GPIO controller of the sl28 board management
> controller. This driver is part of a multi-function device.
> 
> A controller has 8 lines. There are three different flavors:
> full-featured GPIO with interrupt support, input-only and output-only.

FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Michael Walle <michael@walle.cc>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Changes since v5:
>  - added "select REGMAP_IRQ"
> 
> Changes since v4:
>  - update copyright year
>  - remove #include <linux/of_device.h>, suggested by Andy.
>  - use device_get_match_data(), suggested by Andy.
>  - drop the irq_support variable, instead call _init_irq() directly,
>    suggested by Andy.
>  - also move the irq code a bit around to make it look nicer
>  - drop "struct sl28cpld_gpio". We don't need to actually store the
>    irq_chip and irq_chip_data, everything is device resource managed.
>  - use 100 chars line limit, suggested by Andy.
>  - remove the platform device table
>  - don't use KBUID_MODNAME
> 
> Changes since v3:
>  - see cover letter
> 
>  drivers/gpio/Kconfig         |  12 +++
>  drivers/gpio/Makefile        |   1 +
>  drivers/gpio/gpio-sl28cpld.c | 161 +++++++++++++++++++++++++++++++++++
>  3 files changed, 174 insertions(+)
>  create mode 100644 drivers/gpio/gpio-sl28cpld.c
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 8030fd91a3cc..f6a547b76432 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1223,6 +1223,18 @@ config GPIO_RC5T583
>  	  This driver provides the support for driving/reading the gpio pins
>  	  of RC5T583 device through standard gpio library.
>  
> +config GPIO_SL28CPLD
> +	tristate "Kontron sl28cpld GPIO support"
> +	select MFD_SIMPLE_MFD_I2C
> +	select GPIO_REGMAP
> +	select GPIOLIB_IRQCHIP
> +	select REGMAP_IRQ
> +	help
> +	  This enables support for the GPIOs found on the Kontron sl28 CPLD.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called gpio-sl28cpld.
> +
>  config GPIO_STMPE
>  	bool "STMPE GPIOs"
>  	depends on MFD_STMPE
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 4f9abff4f2dc..c3a4e7c94a91 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -132,6 +132,7 @@ obj-$(CONFIG_GPIO_SCH311X)		+= gpio-sch311x.o
>  obj-$(CONFIG_GPIO_SCH)			+= gpio-sch.o
>  obj-$(CONFIG_GPIO_SIFIVE)		+= gpio-sifive.o
>  obj-$(CONFIG_GPIO_SIOX)			+= gpio-siox.o
> +obj-$(CONFIG_GPIO_SL28CPLD)		+= gpio-sl28cpld.o
>  obj-$(CONFIG_GPIO_SODAVILLE)		+= gpio-sodaville.o
>  obj-$(CONFIG_GPIO_SPEAR_SPICS)		+= gpio-spear-spics.o
>  obj-$(CONFIG_GPIO_SPRD)			+= gpio-sprd.o
> diff --git a/drivers/gpio/gpio-sl28cpld.c b/drivers/gpio/gpio-sl28cpld.c
> new file mode 100644
> index 000000000000..889b8f5622c2
> --- /dev/null
> +++ b/drivers/gpio/gpio-sl28cpld.c
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * sl28cpld GPIO driver
> + *
> + * Copyright 2020 Michael Walle <michael@walle.cc>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/gpio/regmap.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +/* GPIO flavor */
> +#define GPIO_REG_DIR	0x00
> +#define GPIO_REG_OUT	0x01
> +#define GPIO_REG_IN	0x02
> +#define GPIO_REG_IE	0x03
> +#define GPIO_REG_IP	0x04
> +
> +/* input-only flavor */
> +#define GPI_REG_IN	0x00
> +
> +/* output-only flavor */
> +#define GPO_REG_OUT	0x00
> +
> +enum sl28cpld_gpio_type {
> +	SL28CPLD_GPIO = 1,
> +	SL28CPLD_GPI,
> +	SL28CPLD_GPO,
> +};
> +
> +static const struct regmap_irq sl28cpld_gpio_irqs[] = {
> +	REGMAP_IRQ_REG_LINE(0, 8),
> +	REGMAP_IRQ_REG_LINE(1, 8),
> +	REGMAP_IRQ_REG_LINE(2, 8),
> +	REGMAP_IRQ_REG_LINE(3, 8),
> +	REGMAP_IRQ_REG_LINE(4, 8),
> +	REGMAP_IRQ_REG_LINE(5, 8),
> +	REGMAP_IRQ_REG_LINE(6, 8),
> +	REGMAP_IRQ_REG_LINE(7, 8),
> +};
> +
> +static int sl28cpld_gpio_irq_init(struct platform_device *pdev,
> +				  unsigned int base,
> +				  struct gpio_regmap_config *config)
> +{
> +	struct regmap_irq_chip_data *irq_data;
> +	struct regmap_irq_chip *irq_chip;
> +	struct device *dev = &pdev->dev;
> +	int irq, ret;
> +
> +	if (!device_property_read_bool(dev, "interrupt-controller"))
> +		return 0;
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return irq;
> +
> +	irq_chip = devm_kzalloc(dev, sizeof(*irq_chip), GFP_KERNEL);
> +	if (!irq_chip)
> +		return -ENOMEM;
> +
> +	irq_chip->name = "sl28cpld-gpio-irq",
> +	irq_chip->irqs = sl28cpld_gpio_irqs;
> +	irq_chip->num_irqs = ARRAY_SIZE(sl28cpld_gpio_irqs);
> +	irq_chip->num_regs = 1;
> +	irq_chip->status_base = base + GPIO_REG_IP;
> +	irq_chip->mask_base = base + GPIO_REG_IE;
> +	irq_chip->mask_invert = true,
> +	irq_chip->ack_base = base + GPIO_REG_IP;
> +
> +	ret = devm_regmap_add_irq_chip_fwnode(dev, dev_fwnode(dev),
> +					      config->regmap, irq,
> +					      IRQF_SHARED | IRQF_ONESHOT,
> +					      0, irq_chip, &irq_data);
> +	if (ret)
> +		return ret;
> +
> +	config->irq_domain = regmap_irq_get_domain(irq_data);
> +
> +	return 0;
> +}
> +
> +static int sl28cpld_gpio_probe(struct platform_device *pdev)
> +{
> +	struct gpio_regmap_config config = {0};
> +	enum sl28cpld_gpio_type type;
> +	struct regmap *regmap;
> +	u32 base;
> +	int ret;
> +
> +	if (!pdev->dev.parent)
> +		return -ENODEV;
> +
> +	type = (uintptr_t)device_get_match_data(&pdev->dev);
> +	if (!type)
> +		return -ENODEV;
> +
> +	ret = device_property_read_u32(&pdev->dev, "reg", &base);
> +	if (ret)
> +		return -EINVAL;
> +
> +	regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!regmap)
> +		return -ENODEV;
> +
> +	config.regmap = regmap;
> +	config.parent = &pdev->dev;
> +	config.ngpio = 8;
> +
> +	switch (type) {
> +	case SL28CPLD_GPIO:
> +		config.reg_dat_base = base + GPIO_REG_IN;
> +		config.reg_set_base = base + GPIO_REG_OUT;
> +		/* reg_dir_out_base might be zero */
> +		config.reg_dir_out_base = GPIO_REGMAP_ADDR(base + GPIO_REG_DIR);
> +
> +		/* This type supports interrupts */
> +		ret = sl28cpld_gpio_irq_init(pdev, base, &config);
> +		if (ret)
> +			return ret;
> +		break;
> +	case SL28CPLD_GPO:
> +		config.reg_set_base = base + GPO_REG_OUT;
> +		break;
> +	case SL28CPLD_GPI:
> +		config.reg_dat_base = base + GPI_REG_IN;
> +		break;
> +	default:
> +		dev_err(&pdev->dev, "unknown type %d\n", type);
> +		return -ENODEV;
> +	}
> +
> +	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(&pdev->dev, &config));
> +}
> +
> +static const struct of_device_id sl28cpld_gpio_of_match[] = {
> +	{ .compatible = "kontron,sl28cpld-gpio", .data = (void *)SL28CPLD_GPIO },
> +	{ .compatible = "kontron,sl28cpld-gpi", .data = (void *)SL28CPLD_GPI },
> +	{ .compatible = "kontron,sl28cpld-gpo", .data = (void *)SL28CPLD_GPO },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, sl28cpld_gpio_of_match);
> +
> +static struct platform_driver sl28cpld_gpio_driver = {
> +	.probe = sl28cpld_gpio_probe,
> +	.driver = {
> +		.name = "sl28cpld-gpio",
> +		.of_match_table = sl28cpld_gpio_of_match,
> +	},
> +};
> +module_platform_driver(sl28cpld_gpio_driver);
> +
> +MODULE_DESCRIPTION("sl28cpld GPIO Driver");
> +MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.20.1
> 

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v6 08/13] hwmon: add support for the sl28cpld hardware monitoring controller
  2020-07-25 23:18 ` [PATCH v6 08/13] hwmon: add support for the sl28cpld hardware monitoring controller Michael Walle
@ 2020-07-26  9:18   ` Andy Shevchenko
  0 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2020-07-26  9:18 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Catalin Marinas, Will Deacon, Pavel Machek

On Sun, Jul 26, 2020 at 01:18:29AM +0200, Michael Walle wrote:
> Add support for the hardware monitoring controller of the sl28cpld board
> management controller. This driver is part of a multi-function device.

FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Michael Walle <michael@walle.cc>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Changes since v5:
>  - none
> 
> Changes since v4:
>  - update copyright year
>  - remove #include <linux/of_device.h>, suggested by Andy.
>  - use PTR_ERR_OR_ZERO(), suggested by Andy.
>  - remove the platform device table
>  - don't use KBUID_MODNAME
> 
> Changes since v3:
>  - see cover letter
> 
>  Documentation/hwmon/index.rst    |   1 +
>  Documentation/hwmon/sl28cpld.rst |  36 ++++++++
>  drivers/hwmon/Kconfig            |  10 +++
>  drivers/hwmon/Makefile           |   1 +
>  drivers/hwmon/sl28cpld-hwmon.c   | 142 +++++++++++++++++++++++++++++++
>  5 files changed, 190 insertions(+)
>  create mode 100644 Documentation/hwmon/sl28cpld.rst
>  create mode 100644 drivers/hwmon/sl28cpld-hwmon.c
> 
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 750d3a975d82..d90c43c82936 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -154,6 +154,7 @@ Hardware Monitoring Kernel Drivers
>     sht3x
>     shtc1
>     sis5595
> +   sl28cpld
>     smm665
>     smsc47b397
>     smsc47m192
> diff --git a/Documentation/hwmon/sl28cpld.rst b/Documentation/hwmon/sl28cpld.rst
> new file mode 100644
> index 000000000000..7ed65f78250c
> --- /dev/null
> +++ b/Documentation/hwmon/sl28cpld.rst
> @@ -0,0 +1,36 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +Kernel driver sl28cpld
> +======================
> +
> +Supported chips:
> +
> +   * Kontron sl28cpld
> +
> +     Prefix: 'sl28cpld'
> +
> +     Datasheet: not available
> +
> +Authors: Michael Walle <michael@walle.cc>
> +
> +Description
> +-----------
> +
> +The sl28cpld is a board management controller which also exposes a hardware
> +monitoring controller. At the moment this controller supports a single fan
> +supervisor. In the future there might be other flavours and additional
> +hardware monitoring might be supported.
> +
> +The fan supervisor has a 7 bit counter register and a counter period of 1
> +second. If the 7 bit counter overflows, the supervisor will automatically
> +switch to x8 mode to support a wider input range at the loss of
> +granularity.
> +
> +Sysfs entries
> +-------------
> +
> +The following attributes are supported.
> +
> +======================= ========================================================
> +fan1_input		Fan RPM. Assuming 2 pulses per revolution.
> +======================= ========================================================
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 8dc28b26916e..a02829ec7386 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1479,6 +1479,16 @@ config SENSORS_RASPBERRYPI_HWMON
>  	  This driver can also be built as a module. If so, the module
>  	  will be called raspberrypi-hwmon.
>  
> +config SENSORS_SL28CPLD
> +	tristate "Kontron sl28cpld hardware monitoring driver"
> +	select MFD_SIMPLE_MFD_I2C
> +	help
> +	  If you say yes here you get support for the fan supervisor of the
> +	  sl28cpld board management controller.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called sl28cpld-hwmon.
> +
>  config SENSORS_SHT15
>  	tristate "Sensiron humidity and temperature sensors. SHT15 and compat."
>  	depends on GPIOLIB || COMPILE_TEST
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index a8f4b35b136b..dee8511f9348 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -159,6 +159,7 @@ obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
>  obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
>  obj-$(CONFIG_SENSORS_SCH5627)	+= sch5627.o
>  obj-$(CONFIG_SENSORS_SCH5636)	+= sch5636.o
> +obj-$(CONFIG_SENSORS_SL28CPLD)	+= sl28cpld-hwmon.o
>  obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
>  obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
>  obj-$(CONFIG_SENSORS_SHT3x)	+= sht3x.o
> diff --git a/drivers/hwmon/sl28cpld-hwmon.c b/drivers/hwmon/sl28cpld-hwmon.c
> new file mode 100644
> index 000000000000..e48f58ec5b9c
> --- /dev/null
> +++ b/drivers/hwmon/sl28cpld-hwmon.c
> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * sl28cpld hardware monitoring driver
> + *
> + * Copyright 2020 Kontron Europe GmbH
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/hwmon.h>
> +#include <linux/kernel.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +#define FAN_INPUT		0x00
> +#define   FAN_SCALE_X8		BIT(7)
> +#define   FAN_VALUE_MASK	GENMASK(6, 0)
> +
> +struct sl28cpld_hwmon {
> +	struct regmap *regmap;
> +	u32 offset;
> +};
> +
> +static umode_t sl28cpld_hwmon_is_visible(const void *data,
> +					 enum hwmon_sensor_types type,
> +					 u32 attr, int channel)
> +{
> +	return 0444;
> +}
> +
> +static int sl28cpld_hwmon_read(struct device *dev,
> +			       enum hwmon_sensor_types type, u32 attr,
> +			       int channel, long *input)
> +{
> +	struct sl28cpld_hwmon *hwmon = dev_get_drvdata(dev);
> +	unsigned int value;
> +	int ret;
> +
> +	switch (attr) {
> +	case hwmon_fan_input:
> +		ret = regmap_read(hwmon->regmap, hwmon->offset + FAN_INPUT,
> +				  &value);
> +		if (ret)
> +			return ret;
> +		/*
> +		 * The register has a 7 bit value and 1 bit which indicates the
> +		 * scale. If the MSB is set, then the lower 7 bit has to be
> +		 * multiplied by 8, to get the correct reading.
> +		 */
> +		if (value & FAN_SCALE_X8)
> +			value = FIELD_GET(FAN_VALUE_MASK, value) << 3;
> +
> +		/*
> +		 * The counter period is 1000ms and the sysfs specification
> +		 * says we should asssume 2 pulses per revolution.
> +		 */
> +		value *= 60 / 2;
> +
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	*input = value;
> +	return 0;
> +}
> +
> +static const u32 sl28cpld_hwmon_fan_config[] = {
> +	HWMON_F_INPUT,
> +	0
> +};
> +
> +static const struct hwmon_channel_info sl28cpld_hwmon_fan = {
> +	.type = hwmon_fan,
> +	.config = sl28cpld_hwmon_fan_config,
> +};
> +
> +static const struct hwmon_channel_info *sl28cpld_hwmon_info[] = {
> +	&sl28cpld_hwmon_fan,
> +	NULL
> +};
> +
> +static const struct hwmon_ops sl28cpld_hwmon_ops = {
> +	.is_visible = sl28cpld_hwmon_is_visible,
> +	.read = sl28cpld_hwmon_read,
> +};
> +
> +static const struct hwmon_chip_info sl28cpld_hwmon_chip_info = {
> +	.ops = &sl28cpld_hwmon_ops,
> +	.info = sl28cpld_hwmon_info,
> +};
> +
> +static int sl28cpld_hwmon_probe(struct platform_device *pdev)
> +{
> +	struct sl28cpld_hwmon *hwmon;
> +	struct device *hwmon_dev;
> +	int ret;
> +
> +	if (!pdev->dev.parent)
> +		return -ENODEV;
> +
> +	hwmon = devm_kzalloc(&pdev->dev, sizeof(*hwmon), GFP_KERNEL);
> +	if (!hwmon)
> +		return -ENOMEM;
> +
> +	hwmon->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!hwmon->regmap)
> +		return -ENODEV;
> +
> +	ret = device_property_read_u32(&pdev->dev, "reg", &hwmon->offset);
> +	if (ret)
> +		return -EINVAL;
> +
> +	hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
> +				"sl28cpld_hwmon", hwmon,
> +				&sl28cpld_hwmon_chip_info, NULL);
> +	if (IS_ERR(hwmon_dev))
> +		dev_err(&pdev->dev, "failed to register as hwmon device");
> +
> +	return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +static const struct of_device_id sl28cpld_hwmon_of_match[] = {
> +	{ .compatible = "kontron,sl28cpld-fan" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, sl28cpld_hwmon_of_match);
> +
> +static struct platform_driver sl28cpld_hwmon_driver = {
> +	.probe = sl28cpld_hwmon_probe,
> +	.driver = {
> +		.name = "sl28cpld-fan",
> +		.of_match_table = sl28cpld_hwmon_of_match,
> +	},
> +};
> +module_platform_driver(sl28cpld_hwmon_driver);
> +
> +MODULE_DESCRIPTION("sl28cpld Hardware Monitoring Driver");
> +MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.20.1
> 

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v6 06/13] pwm: add support for sl28cpld PWM controller
  2020-07-25 23:18 ` [PATCH v6 06/13] pwm: add support for sl28cpld PWM controller Michael Walle
@ 2020-07-27  7:30   ` Thierry Reding
  2020-07-28  7:43   ` Uwe Kleine-König
  1 sibling, 0 replies; 35+ messages in thread
From: Thierry Reding @ 2020-07-27  7:30 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Lee Jones, Uwe Kleine-König, Wim Van Sebroeck, Shawn Guo,
	Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier, Mark Brown,
	Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek

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

On Sun, Jul 26, 2020 at 01:18:27AM +0200, Michael Walle wrote:
> Add support for the PWM controller of the sl28cpld board management
> controller. This is part of a multi-function device driver.
> 
> The controller has one PWM channel and can just generate four distinct
> frequencies.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> Changes since v5:
>  - added brief description of the PWM hardware implementation
>  - added hardware limitations
>  - dropped the frequency mode table, instead calculate the prescaler
>    value on the fly.
>  - round the requested parameters instead of support just distinct
>    periods.
>  - prefix the macros by SL28CPLD_ to make them less generic
>  - set polarity to PWM_POLARITY_NORMAL and reject inverted polarity
>    requests.
>  - apply the workaround just for prescaler value of 0.
>  - make errors during probing more verbose
> 
> Changes since v4:
>  - update copyright year
>  - remove #include <linux/of_device.h>, suggested by Andy.
>  - make the pwm mode table look nicer, suggested by Lee.
>  - use dev_get_drvdata(chip->dev) instead of container_of(), suggested by
>    Lee.
>  - use whole sentence in comments, suggested by Lee.
>  - renamed the local "struct sl28cpld_pwm" variable to "priv" everywhere,
>    suggested by Lee.
>  - use pwm_{get,set}_relative_duty_cycle(), suggested by Andy.
>  - make the comment about the 250Hz hardware limitation clearer
>  - don't use "if (ret < 0)", but only "if (ret)", suggested by Andy.
>  - don't use KBUID_MODNAME
>  - remove comma in terminator line of the compatible strings list
>  - remove the platform device table
> 
> Changes since v3:
>  - see cover letter
> 
>  drivers/pwm/Kconfig        |  10 ++
>  drivers/pwm/Makefile       |   1 +
>  drivers/pwm/pwm-sl28cpld.c | 223 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 234 insertions(+)
>  create mode 100644 drivers/pwm/pwm-sl28cpld.c
> 
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 7dbcf6973d33..a0d50d70c3b9 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -428,6 +428,16 @@ config PWM_SIFIVE
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called pwm-sifive.
>  
> +config PWM_SL28CPLD
> +	tristate "Kontron sl28cpld PWM support"
> +	select MFD_SIMPLE_MFD_I2C
> +	help
> +	  Generic PWM framework driver for board management controller
> +	  found on the Kontron sl28 CPLD.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called pwm-sl28cpld.
> +
>  config PWM_SPEAR
>  	tristate "STMicroelectronics SPEAr PWM support"
>  	depends on PLAT_SPEAR || COMPILE_TEST
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 2c2ba0a03557..cbdcd55d69ee 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_PWM_RENESAS_TPU)	+= pwm-renesas-tpu.o
>  obj-$(CONFIG_PWM_ROCKCHIP)	+= pwm-rockchip.o
>  obj-$(CONFIG_PWM_SAMSUNG)	+= pwm-samsung.o
>  obj-$(CONFIG_PWM_SIFIVE)	+= pwm-sifive.o
> +obj-$(CONFIG_PWM_SL28CPLD)	+= pwm-sl28cpld.o
>  obj-$(CONFIG_PWM_SPEAR)		+= pwm-spear.o
>  obj-$(CONFIG_PWM_SPRD)		+= pwm-sprd.o
>  obj-$(CONFIG_PWM_STI)		+= pwm-sti.o
> diff --git a/drivers/pwm/pwm-sl28cpld.c b/drivers/pwm/pwm-sl28cpld.c
> new file mode 100644
> index 000000000000..956fa09f3aba
> --- /dev/null
> +++ b/drivers/pwm/pwm-sl28cpld.c
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * sl28cpld PWM driver
> + *
> + * Copyright (c) 2020 Michael Walle <michael@walle.cc>
> + *
> + * There is no public datasheet available for this PWM core. But it is easy
> + * enough to be briefly explained. It consists of one 8-bit counter. The PWM
> + * supports four distinct frequencies by selecting when to reset the counter.
> + * With the prescaler setting you can select which bit of the counter is used
> + * to reset it. This implies that the higher the frequency the less remaining
> + * bits are available for the actual counter.
> + *
> + * Let cnt[7:0] be the counter, clocked at 32kHz:
> + * +-----------+--------+--------------+-----------+
> + * | prescaler |  reset | counter bits | frequency |
> + * +-----------+--------+--------------+-----------+
> + * |         0 | cnt[7] |     cnt[6:0] |     250Hz |
> + * |         1 | cnt[6] |     cnt[5:0] |     500Hz |
> + * |         2 | cnt[5] |     cnt[4:0] |      1kHz |
> + * |         3 | cnt[4] |     cnt[3:0] |      2kHz |
> + * +-----------+--------+--------------+-----------+
> + *
> + * Limitations:
> + * - The hardware cannot generate a 100% duty cycle if the prescaler is 0.
> + * - The hardware cannot atomically set the prescaler and the counter value,
> + *   which might lead to glitches and inconsistent states if a write fails.
> + * - The counter is not reset if you switch the prescaler which leads
> + *   to glitches, too.
> + * - The duty cycle will switch immediately and not after a complete cycle.
> + * - Depending on the actual implementation, disabling the PWM might have
> + *   side effects. For example, if the output pin is shared with a GPIO pin
> + *   it will automatically switch back to GPIO mode.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/kernel.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
> +
> +/*
> + * PWM timer block registers.
> + */
> +#define SL28CPLD_PWM_CTRL			0x00
> +#define   SL28CPLD_PWM_CTRL_ENABLE		BIT(7)
> +#define   SL28CPLD_PWM_CTRL_PRESCALER_MASK	GENMASK(1, 0)
> +#define SL28CPLD_PWM_CYCLE			0x01
> +#define   SL28CPLD_PWM_CYCLE_MAX		GENMASK(6, 0)
> +
> +#define SL28CPLD_PWM_CLK			32000 /* 32 kHz */
> +#define SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler)	(1 << (7 - (prescaler)))
> +#define SL28CPLD_PWM_PERIOD(prescaler) \
> +	(NSEC_PER_SEC / SL28CPLD_PWM_CLK * SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler))
> +
> +/*
> + * We calculate the duty cycle like this:
> + *   duty_cycle_ns = pwm_cycle_reg * max_period_ns / max_duty_cycle
> + *
> + * With
> + *   max_period_ns = (1 << 7 - prescaler) / pwm_clk * NSEC_PER_SEC
> + *   max_duty_cycle = 1 << (7 - prescaler)
> + * this then simplifies to:
> + *   duty_cycle_ns = pwm_cycle_reg / pwm_clk * NSEC_PER_SEC
> + */
> +#define SL28CPLD_PWM_TO_DUTY_CYCLE(reg) \
> +	(NSEC_PER_SEC / SL28CPLD_PWM_CLK * (reg))
> +#define SL28CPLD_PWM_FROM_DUTY_CYCLE(duty_cycle) \
> +	(DIV_ROUND_DOWN_ULL((duty_cycle), NSEC_PER_SEC / SL28CPLD_PWM_CLK))
> +
> +struct sl28cpld_pwm {
> +	struct pwm_chip pwm_chip;
> +	struct regmap *regmap;
> +	u32 offset;
> +};
> +
> +static void sl28cpld_pwm_get_state(struct pwm_chip *chip,
> +				   struct pwm_device *pwm,
> +				   struct pwm_state *state)
> +{
> +	struct sl28cpld_pwm *priv = dev_get_drvdata(chip->dev);
> +	unsigned int reg;
> +	int prescaler;
> +
> +	regmap_read(priv->regmap, priv->offset + SL28CPLD_PWM_CTRL, &reg);
> +
> +	state->enabled = reg & SL28CPLD_PWM_CTRL_ENABLE;
> +
> +	prescaler = FIELD_GET(SL28CPLD_PWM_CTRL_PRESCALER_MASK, reg);
> +	state->period = SL28CPLD_PWM_PERIOD(prescaler);
> +
> +	regmap_read(priv->regmap, priv->offset + SL28CPLD_PWM_CYCLE, &reg);
> +	state->duty_cycle = SL28CPLD_PWM_TO_DUTY_CYCLE(reg);
> +	state->polarity = PWM_POLARITY_NORMAL;
> +}
> +
> +static int sl28cpld_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +			      const struct pwm_state *state)
> +{
> +	struct sl28cpld_pwm *priv = dev_get_drvdata(chip->dev);
> +	unsigned int cycle, prescaler;
> +	int ret;
> +	u8 ctrl;
> +
> +	/* Polarity inversion is not supported */
> +	if (state->polarity != PWM_POLARITY_NORMAL)
> +		return -EINVAL;

Just a note to myself since this just occurred to me: in the legacy API
we used to have a ->set_polarity() callback that indicated whether or
not a controller supports inversion. Since that criterion can no longer
be used with the atomic API we may want to consider adding some sort of
capability flags so that these checks can be performed in the core.

> +
> +	/*
> +	 * Calculate the prescaler. Pick the the biggest period that isn't
> +	 * bigger than the requested period.
> +	 */
> +	prescaler = DIV_ROUND_UP_ULL(SL28CPLD_PWM_PERIOD(0), state->period);
> +	prescaler = order_base_2(prescaler);
> +
> +	if (prescaler > field_max(SL28CPLD_PWM_CTRL_PRESCALER_MASK))
> +		return -ERANGE;
> +
> +	ctrl = FIELD_PREP(SL28CPLD_PWM_CTRL_PRESCALER_MASK, prescaler);
> +	if (state->enabled)
> +		ctrl |= SL28CPLD_PWM_CTRL_ENABLE;
> +
> +	cycle = SL28CPLD_PWM_FROM_DUTY_CYCLE(state->duty_cycle);
> +	cycle = min_t(unsigned int, cycle, SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler));
> +
> +	/*
> +	 * Work around the hardware limitation. See also above. Trap 100% duty
> +	 * cycle if the prescaler is 0. Set prescaler to 1 instead. We don't
> +	 * care about the frequency because its "all-one" in either case.
> +	 *
> +	 * We don't need to check the actual prescaler setting, because only
> +	 * if the prescaler is 0 we can have this particular value.
> +	 */
> +	if (cycle == SL28CPLD_PWM_MAX_DUTY_CYCLE(0)) {
> +		ctrl &= ~SL28CPLD_PWM_CTRL_PRESCALER_MASK;
> +		ctrl |= FIELD_PREP(SL28CPLD_PWM_CTRL_PRESCALER_MASK, 1);
> +		cycle = SL28CPLD_PWM_MAX_DUTY_CYCLE(1);
> +	}
> +
> +	ret = regmap_write(priv->regmap, priv->offset + SL28CPLD_PWM_CTRL, ctrl);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_write(priv->regmap, priv->offset + SL28CPLD_PWM_CYCLE, (u8)cycle);
> +}
> +
> +static const struct pwm_ops sl28cpld_pwm_ops = {
> +	.apply = sl28cpld_pwm_apply,
> +	.get_state = sl28cpld_pwm_get_state,
> +	.owner = THIS_MODULE,
> +};
> +
> +static int sl28cpld_pwm_probe(struct platform_device *pdev)
> +{
> +	struct sl28cpld_pwm *priv;
> +	struct pwm_chip *chip;
> +	int ret;
> +
> +	if (!pdev->dev.parent)
> +		return -ENODEV;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!priv->regmap)
> +		return -ENODEV;
> +
> +	ret = device_property_read_u32(&pdev->dev, "reg", &priv->offset);
> +	if (ret) {
> +		dev_err(&pdev->dev, "no 'reg' property found (%pe)\n",
> +			ERR_PTR(ret));
> +		return -EINVAL;
> +	}
> +
> +	/* Initialize the pwm_chip structure */
> +	chip = &priv->pwm_chip;
> +	chip->dev = &pdev->dev;
> +	chip->ops = &sl28cpld_pwm_ops;
> +	chip->base = -1;
> +	chip->npwm = 1;
> +
> +	ret = pwmchip_add(&priv->pwm_chip);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to add PWM chip (%pe)",
> +			ERR_PTR(ret));
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	return 0;
> +}
> +
> +static int sl28cpld_pwm_remove(struct platform_device *pdev)
> +{
> +	struct sl28cpld_pwm *priv = platform_get_drvdata(pdev);
> +
> +	return pwmchip_remove(&priv->pwm_chip);
> +}
> +
> +static const struct of_device_id sl28cpld_pwm_of_match[] = {
> +	{ .compatible = "kontron,sl28cpld-pwm" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, sl28cpld_pwm_of_match);
> +
> +static struct platform_driver sl28cpld_pwm_driver = {
> +	.probe = sl28cpld_pwm_probe,
> +	.remove	= sl28cpld_pwm_remove,
> +	.driver = {
> +		.name = "sl28cpld-pwm",
> +		.of_match_table = sl28cpld_pwm_of_match,
> +	},
> +};
> +module_platform_driver(sl28cpld_pwm_driver);
> +
> +MODULE_DESCRIPTION("sl28cpld PWM Driver");
> +MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
> +MODULE_LICENSE("GPL");

Looks good to me. I assume Lee will want to merge this along with the
other changes through the MFD tree? If so:

Acked-by: Thierry Reding <thierry.reding@gmail.com>

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

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

* Re: [PATCH v6 01/13] mfd: add simple regmap based I2C driver
  2020-07-25 23:18 ` [PATCH v6 01/13] mfd: add simple regmap based I2C driver Michael Walle
@ 2020-07-28  7:19   ` Lee Jones
  2020-07-28  7:42     ` Michael Walle
  0 siblings, 1 reply; 35+ messages in thread
From: Lee Jones @ 2020-07-28  7:19 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek

On Sun, 26 Jul 2020, Michael Walle wrote:

> There are I2C devices which contain several different functions but
> doesn't require any special access functions. For these kind of drivers
> an I2C regmap should be enough.
> 
> Create an I2C driver which creates an I2C regmap and enumerates its
> children. If a device wants to use this as its MFD core driver, it has
> to add an individual compatible string. It may provide its own regmap
> configuration.
> 
> Subdevices can use dev_get_regmap() on the parent to get their regmap
> instance.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> Changes since v5:
>  - removed "select MFD_CORE" in Kconfig
>  - removed help text in Kconfig, we assume that the users of this

That's the opposite of what I asked for.

>    driver will have a "select MFD_SIMPLE_MFD_I2C". Instead added
>    a small description to the driver itself.
>  - removed "struct simple_mfd_i2c_config" and use regmap_config
>    directly
>  - changed builtin_i2c_driver() to module_i2c_driver(), added
>    MODULE_ boilerplate
>  - cleaned up the included files
> 
> Changes since v4:
>  - new patch. Lee, please bear with me. I didn't want to delay the
>    new version (where a lot of remarks on the other patches were
>    addressed) even more, just because we haven't figured out how
>    to deal with the MFD part. So for now, I've included this one.
> 
>  drivers/mfd/Kconfig          |  5 ++++
>  drivers/mfd/Makefile         |  1 +
>  drivers/mfd/simple-mfd-i2c.c | 55 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 61 insertions(+)
>  create mode 100644 drivers/mfd/simple-mfd-i2c.c
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 33df0837ab41..c08539c7a166 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1162,6 +1162,11 @@ config MFD_SI476X_CORE
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called si476x-core.
>  
> +config MFD_SIMPLE_MFD_I2C
> +	tristate
> +	depends on I2C
> +	select REGMAP_I2C

Please provide a full help.

>  config MFD_SM501
>  	tristate "Silicon Motion SM501"
>  	depends on HAS_DMA
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index a60e5f835283..78d24a3e7c9e 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -264,3 +264,4 @@ obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
>  obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
>  
>  obj-$(CONFIG_SGI_MFD_IOC3)	+= ioc3.o
> +obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)	+= simple-mfd-i2c.o
> diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> new file mode 100644
> index 000000000000..45090ddad104
> --- /dev/null
> +++ b/drivers/mfd/simple-mfd-i2c.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * A very simple I2C MFD driver

Simple MFD - I2C

> + * The driver enumerates its children and registers a common regmap. Use
> + * dev_get_regmap(pdev->dev.parent, NULL) in the child nodes to fetch that
> + * regmap instance.

This driver creates a single register map with the intention for it to
be shared by all sub-devices.  Children can use their parent's device
structure (dev.parent) in order reference it. 

> + * In the future this driver might be extended to support also other interfaces
> + * like SPI etc.

Remove this please.

> + */

'\n' here.

> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/regmap.h>
> +
> +static const struct regmap_config simple_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +};
> +
> +static int simple_mfd_i2c_probe(struct i2c_client *i2c)
> +{
> +	const struct regmap_config *config;
> +	struct regmap *regmap;
> +
> +	config = device_get_match_data(&i2c->dev);
> +	if (!config)
> +		config = &simple_regmap_config;
> +
> +	regmap = devm_regmap_init_i2c(i2c, config);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	return devm_of_platform_populate(&i2c->dev);
> +}
> +
> +static const struct of_device_id simple_mfd_i2c_of_match[] = {
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
> +
> +static struct i2c_driver simple_mfd_i2c_driver = {
> +	.probe_new = simple_mfd_i2c_probe,
> +	.driver = {
> +		.name = "simple-mfd-i2c",
> +		.of_match_table = simple_mfd_i2c_of_match,
> +	},
> +};
> +module_i2c_driver(simple_mfd_i2c_driver);
> +
> +MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
> +MODULE_DESCRIPTION("Simple I2C MFD driver");

Simple MFD - I2C driver

> +MODULE_LICENSE("GPL v2");

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

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

* Re: [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld
  2020-07-25 23:18 ` [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld Michael Walle
@ 2020-07-28  7:24   ` Lee Jones
  2020-07-28  7:57     ` Michael Walle
  0 siblings, 1 reply; 35+ messages in thread
From: Lee Jones @ 2020-07-28  7:24 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek, Rob Herring

On Sun, 26 Jul 2020, Michael Walle wrote:

> Add a device tree bindings for the board management controller found on
> the Kontron SMARC-sAL28 board.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> Changes since v5:
>  - none
> 
> Changes since v4:
>  - fix the regex of the unit-address
> 
> Changes since v3:
>  - see cover letter
> 
>  .../bindings/gpio/kontron,sl28cpld-gpio.yaml  |  54 +++++++
>  .../hwmon/kontron,sl28cpld-hwmon.yaml         |  27 ++++
>  .../kontron,sl28cpld-intc.yaml                |  54 +++++++
>  .../bindings/mfd/kontron,sl28cpld.yaml        | 153 ++++++++++++++++++
>  .../bindings/pwm/kontron,sl28cpld-pwm.yaml    |  35 ++++
>  .../watchdog/kontron,sl28cpld-wdt.yaml        |  35 ++++
>  6 files changed, 358 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
>  create mode 100644 Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
>  create mode 100644 Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
>  create mode 100644 Documentation/devicetree/bindings/pwm/kontron,sl28cpld-pwm.yaml
>  create mode 100644 Documentation/devicetree/bindings/watchdog/kontron,sl28cpld-wdt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml b/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
> new file mode 100644
> index 000000000000..9a63a158a796
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/kontron,sl28cpld-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GPIO driver for the sl28cpld board management controller
> +
> +maintainers:
> +  - Michael Walle <michael@walle.cc>
> +
> +description: |
> +  This module is part of the sl28cpld multi-function device. For more
> +  details see Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.

Paths are normally relative.

> +  There are three flavors of the GPIO controller, one full featured
> +  input/output with interrupt support (kontron,sl28cpld-gpio), one
> +  output-only (kontron,sl28-gpo) and one input-only (kontron,sl28-gpi).
> +
> +  Each controller supports 8 GPIO lines.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - kontron,sl28cpld-gpio
> +      - kontron,sl28cpld-gpi
> +      - kontron,sl28cpld-gpo
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  interrupt-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-controller: true
> +
> +  gpio-line-names:
> +      minItems: 1
> +      maxItems: 8
> +
> +required:
> +  - compatible
> +  - "#gpio-cells"
> +  - gpio-controller
> +
> +additionalProperties: false
> diff --git a/Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
> new file mode 100644
> index 000000000000..1cebd61c6c32
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
> @@ -0,0 +1,27 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/kontron,sl28cpld-hwmon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Hardware monitoring driver for the sl28cpld board management controller
> +
> +maintainers:
> +  - Michael Walle <michael@walle.cc>
> +
> +description: |
> +  This module is part of the sl28cpld multi-function device. For more
> +  details see Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - kontron,sl28cpld-fan
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
> new file mode 100644
> index 000000000000..4c39e9ff9aea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/kontron,sl28cpld-intc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Interrupt controller driver for the sl28cpld board management controller
> +
> +maintainers:
> +  - Michael Walle <michael@walle.cc>
> +
> +description: |
> +  This module is part of the sl28cpld multi-function device. For more
> +  details see Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
> +
> +  The following interrupts are available. All types and levels are fixed
> +  and handled by the board management controller.
> +
> +  ==== ============= ==================================
> +   IRQ line/device   description
> +  ==== ============= ==================================
> +    0  RTC_INT#      Interrupt line from on-board RTC
> +    1  SMB_ALERT#    Event on SMB_ALERT# line (P1)
> +    2  ESPI_ALERT0#  Event on ESPI_ALERT0# line (S43)
> +    3  ESPI_ALERT1#  Event on ESPI_ALERT1# line (S44)
> +    4  PWR_BTN#      Event on PWR_BTN# line (P128)
> +    5  SLEEP#        Event on SLEEP# line (S149)
> +    6  watchdog      Interrupt of the internal watchdog
> +    7  n/a           not used
> +  ==== ============= ==================================
> +
> +properties:
> +  compatible:
> +    enum:
> +      - kontron,sl28cpld-intc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  interrupt-controller: true
> +
> +required:
> +  - compatible
> +  - interrupts
> +  - "#interrupt-cells"
> +  - interrupt-controller
> +
> +additionalProperties: false
> diff --git a/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml b/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
> new file mode 100644
> index 000000000000..e3a62db678e7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
> @@ -0,0 +1,153 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/kontron,sl28cpld.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Kontron's sl28cpld board management controller

"S128CPLD" ?

"Board Management Controller (BMC)" ?

> +maintainers:
> +  - Michael Walle <michael@walle.cc>
> +
> +description: |
> +  The board management controller may contain different IP blocks like
> +  watchdog, fan monitoring, PWM controller, interrupt controller and a
> +  GPIO controller.
> +
> +properties:
> +  compatible:
> +    const: kontron,sl28cpld-r1

We don't usually code revision numbers in compatible strings.

Is there any way to pull this from the H/W?

> +  reg:
> +    description:
> +      I2C device address.
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +patternProperties:
> +  "^gpio(@[0-9a-f]+)?$":
> +    $ref: ../gpio/kontron,sl28cpld-gpio.yaml
> +
> +  "^hwmon(@[0-9a-f]+)?$":
> +    $ref: ../hwmon/kontron,sl28cpld-hwmon.yaml
> +
> +  "^interrupt-controller(@[0-9a-f]+)?$":
> +    $ref: ../interrupt-controller/kontron,sl28cpld-intc.yaml
> +
> +  "^pwm(@[0-9a-f]+)?$":
> +    $ref: ../pwm/kontron,sl28cpld-pwm.yaml
> +
> +  "^watchdog(@[0-9a-f]+)?$":
> +    $ref: ../watchdog/kontron,sl28cpld-wdt.yaml
> +
> +required:
> +  - "#address-cells"
> +  - "#size-cells"
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        sl28cpld@4a {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            compatible = "kontron,sl28cpld-r1";
> +            reg = <0x4a>;

Nit: Could you put the 'reg' and 'compatible' at the top please?

Same for all nodes.

> +            watchdog@4 {
> +                compatible = "kontron,sl28cpld-wdt";
> +                reg = <0x4>;
> +                kontron,assert-wdt-timeout-pin;
> +            };
> +
> +            hwmon@b {
> +                compatible = "kontron,sl28cpld-fan";
> +                reg = <0xb>;
> +            };
> +
> +            pwm@c {
> +                #pwm-cells = <2>;
> +                compatible = "kontron,sl28cpld-pwm";
> +                reg = <0xc>;
> +            };
> +
> +            pwm@e {
> +                #pwm-cells = <2>;
> +                compatible = "kontron,sl28cpld-pwm";
> +                reg = <0xe>;
> +            };
> +
> +            gpio@10 {
> +                compatible = "kontron,sl28cpld-gpio";
> +                reg = <0x10>;
> +                interrupts-extended = <&gpio2 6
> +                               IRQ_TYPE_EDGE_FALLING>;
> +
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +                gpio-line-names = "a", "b", "c";
> +
> +                interrupt-controller;
> +                #interrupt-cells = <2>;
> +            };
> +
> +            gpio@15 {
> +                compatible = "kontron,sl28cpld-gpio";
> +                reg = <0x15>;
> +                interrupts-extended = <&gpio2 6
> +                               IRQ_TYPE_EDGE_FALLING>;
> +
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +
> +                interrupt-controller;
> +                #interrupt-cells = <2>;
> +            };
> +
> +            gpio@1a {
> +                compatible = "kontron,sl28cpld-gpo";
> +                reg = <0x1a>;
> +
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +            };
> +
> +            gpio@1b {
> +                compatible = "kontron,sl28cpld-gpi";
> +                reg = <0x1b>;
> +
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +            };
> +
> +            interrupt-controller@1c {
> +                compatible = "kontron,sl28cpld-intc";
> +                reg = <0x1c>;
> +                interrupts-extended = <&gpio2 6
> +                               IRQ_TYPE_EDGE_FALLING>;
> +
> +                interrupt-controller;
> +                #interrupt-cells = <2>;
> +            };
> +        };
> +    };

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

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

* Re: [PATCH v6 03/13] mfd: simple-mfd-i2c: add sl28cpld support
  2020-07-25 23:18 ` [PATCH v6 03/13] mfd: simple-mfd-i2c: add sl28cpld support Michael Walle
@ 2020-07-28  7:25   ` Lee Jones
  0 siblings, 0 replies; 35+ messages in thread
From: Lee Jones @ 2020-07-28  7:25 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek

On Sun, 26 Jul 2020, Michael Walle wrote:

> Add the core support for the board management controller found on the
> SMARC-sAL28 board.
> 
> At the moment, this controller is used on the Kontron SMARC-sAL28 board.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> Changes since v5:
>  - none
> 
> Changes since v4:
>  - new patch
> 
>  drivers/mfd/simple-mfd-i2c.c | 1 +
>  1 file changed, 1 insertion(+)

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

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

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

* Re: [PATCH v6 01/13] mfd: add simple regmap based I2C driver
  2020-07-28  7:19   ` Lee Jones
@ 2020-07-28  7:42     ` Michael Walle
  2020-07-28  8:15       ` Lee Jones
  0 siblings, 1 reply; 35+ messages in thread
From: Michael Walle @ 2020-07-28  7:42 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek

Am 2020-07-28 09:19, schrieb Lee Jones:
> On Sun, 26 Jul 2020, Michael Walle wrote:
> 
>> There are I2C devices which contain several different functions but
>> doesn't require any special access functions. For these kind of 
>> drivers
>> an I2C regmap should be enough.
>> 
>> Create an I2C driver which creates an I2C regmap and enumerates its
>> children. If a device wants to use this as its MFD core driver, it has
>> to add an individual compatible string. It may provide its own regmap
>> configuration.
>> 
>> Subdevices can use dev_get_regmap() on the parent to get their regmap
>> instance.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>> Changes since v5:
>>  - removed "select MFD_CORE" in Kconfig
>>  - removed help text in Kconfig, we assume that the users of this
> 
> That's the opposite of what I asked for.

What is the use to describe the symbol, if it is not user selectable?
I'm not aware this is done anywhere in the kernel, am I missing
something?

>>    driver will have a "select MFD_SIMPLE_MFD_I2C". Instead added
>>    a small description to the driver itself.
>>  - removed "struct simple_mfd_i2c_config" and use regmap_config
>>    directly
>>  - changed builtin_i2c_driver() to module_i2c_driver(), added
>>    MODULE_ boilerplate
>>  - cleaned up the included files
>> 
>> Changes since v4:
>>  - new patch. Lee, please bear with me. I didn't want to delay the
>>    new version (where a lot of remarks on the other patches were
>>    addressed) even more, just because we haven't figured out how
>>    to deal with the MFD part. So for now, I've included this one.
>> 
>>  drivers/mfd/Kconfig          |  5 ++++
>>  drivers/mfd/Makefile         |  1 +
>>  drivers/mfd/simple-mfd-i2c.c | 55 
>> ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 61 insertions(+)
>>  create mode 100644 drivers/mfd/simple-mfd-i2c.c
>> 
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 33df0837ab41..c08539c7a166 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -1162,6 +1162,11 @@ config MFD_SI476X_CORE
>>  	  To compile this driver as a module, choose M here: the
>>  	  module will be called si476x-core.
>> 
>> +config MFD_SIMPLE_MFD_I2C
>> +	tristate
>> +	depends on I2C
>> +	select REGMAP_I2C
> 
> Please provide a full help.

See above.

> 
>>  config MFD_SM501
>>  	tristate "Silicon Motion SM501"
>>  	depends on HAS_DMA
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index a60e5f835283..78d24a3e7c9e 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -264,3 +264,4 @@ obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
>>  obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
>> 
>>  obj-$(CONFIG_SGI_MFD_IOC3)	+= ioc3.o
>> +obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)	+= simple-mfd-i2c.o
>> diff --git a/drivers/mfd/simple-mfd-i2c.c 
>> b/drivers/mfd/simple-mfd-i2c.c
>> new file mode 100644
>> index 000000000000..45090ddad104
>> --- /dev/null
>> +++ b/drivers/mfd/simple-mfd-i2c.c
>> @@ -0,0 +1,55 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * A very simple I2C MFD driver
> 
> Simple MFD - I2C

ok.

>> + * The driver enumerates its children and registers a common regmap. 
>> Use
>> + * dev_get_regmap(pdev->dev.parent, NULL) in the child nodes to fetch 
>> that
>> + * regmap instance.
> 
> This driver creates a single register map with the intention for it to
> be shared by all sub-devices.  Children can use their parent's device
> structure (dev.parent) in order reference it.

Should this be appended or should it replace my paragraph? If its the 
latter,
the "enumeration of the children" is missing.

>> + * In the future this driver might be extended to support also other 
>> interfaces
>> + * like SPI etc.
> 
> Remove this please.

Why would you remove information about the intention of this driver? If 
someone
looks for a place to implement its SPI/I3C/Slimbus MFD driver this might 
come
in handy.

-michael

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

* Re: [PATCH v6 06/13] pwm: add support for sl28cpld PWM controller
  2020-07-25 23:18 ` [PATCH v6 06/13] pwm: add support for sl28cpld PWM controller Michael Walle
  2020-07-27  7:30   ` Thierry Reding
@ 2020-07-28  7:43   ` Uwe Kleine-König
  2020-07-28  8:21     ` Michael Walle
  1 sibling, 1 reply; 35+ messages in thread
From: Uwe Kleine-König @ 2020-07-28  7:43 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Lee Jones, Thierry Reding, Wim Van Sebroeck, Shawn Guo, Li Yang,
	Thomas Gleixner, Jason Cooper, Marc Zyngier, Mark Brown,
	Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek

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

Hello,

just a few minor issues left:

On Sun, Jul 26, 2020 at 01:18:27AM +0200, Michael Walle wrote:
> diff --git a/drivers/pwm/pwm-sl28cpld.c b/drivers/pwm/pwm-sl28cpld.c
> new file mode 100644
> index 000000000000..956fa09f3aba
> --- /dev/null
> +++ b/drivers/pwm/pwm-sl28cpld.c
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * sl28cpld PWM driver
> + *
> + * Copyright (c) 2020 Michael Walle <michael@walle.cc>
> + *
> + * There is no public datasheet available for this PWM core. But it is easy
> + * enough to be briefly explained. It consists of one 8-bit counter. The PWM
> + * supports four distinct frequencies by selecting when to reset the counter.
> + * With the prescaler setting you can select which bit of the counter is used
> + * to reset it. This implies that the higher the frequency the less remaining
> + * bits are available for the actual counter.
> + *
> + * Let cnt[7:0] be the counter, clocked at 32kHz:
> + * +-----------+--------+--------------+-----------+
> + * | prescaler |  reset | counter bits | frequency |
> + * +-----------+--------+--------------+-----------+
> + * |         0 | cnt[7] |     cnt[6:0] |     250Hz |
> + * |         1 | cnt[6] |     cnt[5:0] |     500Hz |
> + * |         2 | cnt[5] |     cnt[4:0] |      1kHz |
> + * |         3 | cnt[4] |     cnt[3:0] |      2kHz |
> + * +-----------+--------+--------------+-----------+

Very nice. I'd add a "period length" column, as this is what the PWM
core uses.

For your convenience (and as I created that table anyhow for further
checking of the formulas below):

 * +-----------+--------+--------------+-----------+--------+
 * | prescaler |  reset | counter bits | frequency | period |
 * |           |        |              |           | length |
 * +-----------+--------+--------------+-----------+--------+
 * |         0 | cnt[7] |     cnt[6:0] |     250Hz | 4000ns |
 * |         1 | cnt[6] |     cnt[5:0] |     500Hz | 2000ns |
 * |         2 | cnt[5] |     cnt[4:0] |      1kHz | 1000ns |
 * |         3 | cnt[4] |     cnt[3:0] |      2kHz |  500ns |
 * +-----------+--------+--------------+-----------+--------+

> + *
> + * Limitations:
> + * - The hardware cannot generate a 100% duty cycle if the prescaler is 0.
> + * - The hardware cannot atomically set the prescaler and the counter value,
> + *   which might lead to glitches and inconsistent states if a write fails.
> + * - The counter is not reset if you switch the prescaler which leads
> + *   to glitches, too.
> + * - The duty cycle will switch immediately and not after a complete cycle.
> + * - Depending on the actual implementation, disabling the PWM might have
> + *   side effects. For example, if the output pin is shared with a GPIO pin
> + *   it will automatically switch back to GPIO mode.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/kernel.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
> +
> +/*
> + * PWM timer block registers.
> + */
> +#define SL28CPLD_PWM_CTRL			0x00
> +#define   SL28CPLD_PWM_CTRL_ENABLE		BIT(7)
> +#define   SL28CPLD_PWM_CTRL_PRESCALER_MASK	GENMASK(1, 0)
> +#define SL28CPLD_PWM_CYCLE			0x01
> +#define   SL28CPLD_PWM_CYCLE_MAX		GENMASK(6, 0)
> +
> +#define SL28CPLD_PWM_CLK			32000 /* 32 kHz */
> +#define SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler)	(1 << (7 - (prescaler)))
> +#define SL28CPLD_PWM_PERIOD(prescaler) \
> +	(NSEC_PER_SEC / SL28CPLD_PWM_CLK * SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler))
> +
> +/*
> + * We calculate the duty cycle like this:
> + *   duty_cycle_ns = pwm_cycle_reg * max_period_ns / max_duty_cycle
> + *
> + * With
> + *   max_period_ns = (1 << 7 - prescaler) / pwm_clk * NSEC_PER_SEC
> + *   max_duty_cycle = 1 << (7 - prescaler)

If you don't need parenthesis in the max_period_ns around 7 - prescaler,
you don't need them either in the max_duty_cycle line.

> + * this then simplifies to:
> + *   duty_cycle_ns = pwm_cycle_reg / pwm_clk * NSEC_PER_SEC
> + */
> +#define SL28CPLD_PWM_TO_DUTY_CYCLE(reg) \
> +	(NSEC_PER_SEC / SL28CPLD_PWM_CLK * (reg))

For those who copy from your driver maybe add a comment like:

 * NSEC_PER_SEC / SL28CPLD_PWM_CLK is integer here, so we're not loosing
 * precision by doing the division first.

> +#define SL28CPLD_PWM_FROM_DUTY_CYCLE(duty_cycle) \
> +	(DIV_ROUND_DOWN_ULL((duty_cycle), NSEC_PER_SEC / SL28CPLD_PWM_CLK))
> +
> +struct sl28cpld_pwm {
> +	struct pwm_chip pwm_chip;
> +	struct regmap *regmap;
> +	u32 offset;
> +};
> +
> +static void sl28cpld_pwm_get_state(struct pwm_chip *chip,
> +				   struct pwm_device *pwm,
> +				   struct pwm_state *state)
> +{
> +	struct sl28cpld_pwm *priv = dev_get_drvdata(chip->dev);
> +	unsigned int reg;
> +	int prescaler;
> +
> +	regmap_read(priv->regmap, priv->offset + SL28CPLD_PWM_CTRL, &reg);

Would it make sense to hide this using e.g.:

	#define sl28cpkd_pwm_read(priv, reg, val)	regmap_read((priv)->regmap, (priv)->offset + (reg), val)

The line would then become:

	sl28cpkd_pwm_read(priv, SL28CPLD_PWM_CTRL, &reg);

which is a bit prettier. Up to you to decide. If you do it, please do
the same for write 

> +	state->enabled = reg & SL28CPLD_PWM_CTRL_ENABLE;
> +
> +	prescaler = FIELD_GET(SL28CPLD_PWM_CTRL_PRESCALER_MASK, reg);
> +	state->period = SL28CPLD_PWM_PERIOD(prescaler);
> +
> +	regmap_read(priv->regmap, priv->offset + SL28CPLD_PWM_CYCLE, &reg);
> +	state->duty_cycle = SL28CPLD_PWM_TO_DUTY_CYCLE(reg);
> +	state->polarity = PWM_POLARITY_NORMAL;
> +}
> +
> +static int sl28cpld_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +			      const struct pwm_state *state)
> +{
> +	struct sl28cpld_pwm *priv = dev_get_drvdata(chip->dev);
> +	unsigned int cycle, prescaler;
> +	int ret;
> +	u8 ctrl;
> +
> +	/* Polarity inversion is not supported */
> +	if (state->polarity != PWM_POLARITY_NORMAL)
> +		return -EINVAL;
> +
> +	/*
> +	 * Calculate the prescaler. Pick the the biggest period that isn't
> +	 * bigger than the requested period.
> +	 */
> +	prescaler = DIV_ROUND_UP_ULL(SL28CPLD_PWM_PERIOD(0), state->period);
> +	prescaler = order_base_2(prescaler);
> +
> +	if (prescaler > field_max(SL28CPLD_PWM_CTRL_PRESCALER_MASK))
> +		return -ERANGE;

The calculation looks right.
Did you check the generated code? Maybe using an if or switch here is
more effective? (optional task for bonus points :-)

> +	ctrl = FIELD_PREP(SL28CPLD_PWM_CTRL_PRESCALER_MASK, prescaler);
> +	if (state->enabled)
> +		ctrl |= SL28CPLD_PWM_CTRL_ENABLE;
> +
> +	cycle = SL28CPLD_PWM_FROM_DUTY_CYCLE(state->duty_cycle);
> +	cycle = min_t(unsigned int, cycle, SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler));
> +
> +	/*
> +	 * Work around the hardware limitation. See also above. Trap 100% duty
> +	 * cycle if the prescaler is 0. Set prescaler to 1 instead. We don't
> +	 * care about the frequency because its "all-one" in either case.
> +	 *
> +	 * We don't need to check the actual prescaler setting, because only
> +	 * if the prescaler is 0 we can have this particular value.
> +	 */
> +	if (cycle == SL28CPLD_PWM_MAX_DUTY_CYCLE(0)) {
> +		ctrl &= ~SL28CPLD_PWM_CTRL_PRESCALER_MASK;
> +		ctrl |= FIELD_PREP(SL28CPLD_PWM_CTRL_PRESCALER_MASK, 1);
> +		cycle = SL28CPLD_PWM_MAX_DUTY_CYCLE(1);
> +	}
> +
> +	ret = regmap_write(priv->regmap, priv->offset + SL28CPLD_PWM_CTRL, ctrl);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_write(priv->regmap, priv->offset + SL28CPLD_PWM_CYCLE, (u8)cycle);

This cast isn't needed, is it?

> +}
> +
> +static const struct pwm_ops sl28cpld_pwm_ops = {
> +	.apply = sl28cpld_pwm_apply,
> +	.get_state = sl28cpld_pwm_get_state,
> +	.owner = THIS_MODULE,
> +};
> +
> +static int sl28cpld_pwm_probe(struct platform_device *pdev)
> +{
> +	struct sl28cpld_pwm *priv;
> +	struct pwm_chip *chip;
> +	int ret;
> +
> +	if (!pdev->dev.parent)
> +		return -ENODEV;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!priv->regmap)

Error message here?

> +		return -ENODEV;
> +
> +	ret = device_property_read_u32(&pdev->dev, "reg", &priv->offset);
> +	if (ret) {
> +		dev_err(&pdev->dev, "no 'reg' property found (%pe)\n",
> +			ERR_PTR(ret));
> +		return -EINVAL;
> +	}
> +
> +	/* Initialize the pwm_chip structure */
> +	chip = &priv->pwm_chip;
> +	chip->dev = &pdev->dev;
> +	chip->ops = &sl28cpld_pwm_ops;
> +	chip->base = -1;
> +	chip->npwm = 1;
> +
> +	ret = pwmchip_add(&priv->pwm_chip);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to add PWM chip (%pe)",
> +			ERR_PTR(ret));
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	return 0;
> +}
> +
> +static int sl28cpld_pwm_remove(struct platform_device *pdev)
> +{
> +	struct sl28cpld_pwm *priv = platform_get_drvdata(pdev);
> +
> +	return pwmchip_remove(&priv->pwm_chip);
> +}
> +
> +static const struct of_device_id sl28cpld_pwm_of_match[] = {
> +	{ .compatible = "kontron,sl28cpld-pwm" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, sl28cpld_pwm_of_match);
> +
> +static struct platform_driver sl28cpld_pwm_driver = {
> +	.probe = sl28cpld_pwm_probe,
> +	.remove	= sl28cpld_pwm_remove,
> +	.driver = {
> +		.name = "sl28cpld-pwm",
> +		.of_match_table = sl28cpld_pwm_of_match,
> +	},
> +};
> +module_platform_driver(sl28cpld_pwm_driver);
> +
> +MODULE_DESCRIPTION("sl28cpld PWM Driver");
> +MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
> +MODULE_LICENSE("GPL");

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

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

* Re: [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld
  2020-07-28  7:24   ` Lee Jones
@ 2020-07-28  7:57     ` Michael Walle
  2020-07-28  8:27       ` Lee Jones
  0 siblings, 1 reply; 35+ messages in thread
From: Michael Walle @ 2020-07-28  7:57 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek, Rob Herring

Am 2020-07-28 09:24, schrieb Lee Jones:
> On Sun, 26 Jul 2020, Michael Walle wrote:
> 
>> Add a device tree bindings for the board management controller found 
>> on
>> the Kontron SMARC-sAL28 board.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> ---
>> Changes since v5:
>>  - none
>> 
>> Changes since v4:
>>  - fix the regex of the unit-address
>> 
>> Changes since v3:
>>  - see cover letter
>> 
>>  .../bindings/gpio/kontron,sl28cpld-gpio.yaml  |  54 +++++++
>>  .../hwmon/kontron,sl28cpld-hwmon.yaml         |  27 ++++
>>  .../kontron,sl28cpld-intc.yaml                |  54 +++++++
>>  .../bindings/mfd/kontron,sl28cpld.yaml        | 153 
>> ++++++++++++++++++
>>  .../bindings/pwm/kontron,sl28cpld-pwm.yaml    |  35 ++++
>>  .../watchdog/kontron,sl28cpld-wdt.yaml        |  35 ++++
>>  6 files changed, 358 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
>>  create mode 100644 
>> Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
>>  create mode 100644 
>> Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
>>  create mode 100644 
>> Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
>>  create mode 100644 
>> Documentation/devicetree/bindings/pwm/kontron,sl28cpld-pwm.yaml
>>  create mode 100644 
>> Documentation/devicetree/bindings/watchdog/kontron,sl28cpld-wdt.yaml
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml 
>> b/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
>> new file mode 100644
>> index 000000000000..9a63a158a796
>> --- /dev/null
>> +++ 
>> b/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
>> @@ -0,0 +1,54 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/gpio/kontron,sl28cpld-gpio.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: GPIO driver for the sl28cpld board management controller
>> +
>> +maintainers:
>> +  - Michael Walle <michael@walle.cc>
>> +
>> +description: |
>> +  This module is part of the sl28cpld multi-function device. For more
>> +  details see 
>> Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
> 
> Paths are normally relative.

grep Documentation/ Documentation

I know there are a lot false positives (esp in the first one)..

$ grep -r "\.\./" Documentation | wc -l
1826
$ grep -r "Documentation/" Documentation|wc -l
2862

> 
>> +  There are three flavors of the GPIO controller, one full featured
>> +  input/output with interrupt support (kontron,sl28cpld-gpio), one
>> +  output-only (kontron,sl28-gpo) and one input-only 
>> (kontron,sl28-gpi).
>> +
>> +  Each controller supports 8 GPIO lines.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - kontron,sl28cpld-gpio
>> +      - kontron,sl28cpld-gpi
>> +      - kontron,sl28cpld-gpo
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  "#interrupt-cells":
>> +    const: 2
>> +
>> +  interrupt-controller: true
>> +
>> +  "#gpio-cells":
>> +    const: 2
>> +
>> +  gpio-controller: true
>> +
>> +  gpio-line-names:
>> +      minItems: 1
>> +      maxItems: 8
>> +
>> +required:
>> +  - compatible
>> +  - "#gpio-cells"
>> +  - gpio-controller
>> +
>> +additionalProperties: false
>> diff --git 
>> a/Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml 
>> b/Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
>> new file mode 100644
>> index 000000000000..1cebd61c6c32
>> --- /dev/null
>> +++ 
>> b/Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
>> @@ -0,0 +1,27 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwmon/kontron,sl28cpld-hwmon.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Hardware monitoring driver for the sl28cpld board management 
>> controller
>> +
>> +maintainers:
>> +  - Michael Walle <michael@walle.cc>
>> +
>> +description: |
>> +  This module is part of the sl28cpld multi-function device. For more
>> +  details see 
>> Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - kontron,sl28cpld-fan
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +
>> +additionalProperties: false
>> diff --git 
>> a/Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml 
>> b/Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
>> new file mode 100644
>> index 000000000000..4c39e9ff9aea
>> --- /dev/null
>> +++ 
>> b/Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
>> @@ -0,0 +1,54 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: 
>> http://devicetree.org/schemas/interrupt-controller/kontron,sl28cpld-intc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Interrupt controller driver for the sl28cpld board management 
>> controller
>> +
>> +maintainers:
>> +  - Michael Walle <michael@walle.cc>
>> +
>> +description: |
>> +  This module is part of the sl28cpld multi-function device. For more
>> +  details see 
>> Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
>> +
>> +  The following interrupts are available. All types and levels are 
>> fixed
>> +  and handled by the board management controller.
>> +
>> +  ==== ============= ==================================
>> +   IRQ line/device   description
>> +  ==== ============= ==================================
>> +    0  RTC_INT#      Interrupt line from on-board RTC
>> +    1  SMB_ALERT#    Event on SMB_ALERT# line (P1)
>> +    2  ESPI_ALERT0#  Event on ESPI_ALERT0# line (S43)
>> +    3  ESPI_ALERT1#  Event on ESPI_ALERT1# line (S44)
>> +    4  PWR_BTN#      Event on PWR_BTN# line (P128)
>> +    5  SLEEP#        Event on SLEEP# line (S149)
>> +    6  watchdog      Interrupt of the internal watchdog
>> +    7  n/a           not used
>> +  ==== ============= ==================================
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - kontron,sl28cpld-intc
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  "#interrupt-cells":
>> +    const: 2
>> +
>> +  interrupt-controller: true
>> +
>> +required:
>> +  - compatible
>> +  - interrupts
>> +  - "#interrupt-cells"
>> +  - interrupt-controller
>> +
>> +additionalProperties: false
>> diff --git 
>> a/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml 
>> b/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
>> new file mode 100644
>> index 000000000000..e3a62db678e7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
>> @@ -0,0 +1,153 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mfd/kontron,sl28cpld.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Kontron's sl28cpld board management controller
> 
> "S128CPLD" ?

still not, its sl28cpld, think of a project/code name, not the product
appended with CPLD.

> "Board Management Controller (BMC)" ?

sounds like IPMI, which I wanted to avoid.

> 
>> +maintainers:
>> +  - Michael Walle <michael@walle.cc>
>> +
>> +description: |
>> +  The board management controller may contain different IP blocks 
>> like
>> +  watchdog, fan monitoring, PWM controller, interrupt controller and 
>> a
>> +  GPIO controller.
>> +
>> +properties:
>> +  compatible:
>> +    const: kontron,sl28cpld-r1
> 
> We don't usually code revision numbers in compatible strings.
> 
> Is there any way to pull this from the H/W?

No, unfortunately you can't. And I really want to keep that, in case
in the future there are some backwards incompatible changes.

>> +  reg:
>> +    description:
>> +      I2C device address.
>> +    maxItems: 1
>> +
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 0
>> +
>> +  "#interrupt-cells":
>> +    const: 2
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  interrupt-controller: true
>> +
>> +patternProperties:
>> +  "^gpio(@[0-9a-f]+)?$":
>> +    $ref: ../gpio/kontron,sl28cpld-gpio.yaml
>> +
>> +  "^hwmon(@[0-9a-f]+)?$":
>> +    $ref: ../hwmon/kontron,sl28cpld-hwmon.yaml
>> +
>> +  "^interrupt-controller(@[0-9a-f]+)?$":
>> +    $ref: ../interrupt-controller/kontron,sl28cpld-intc.yaml
>> +
>> +  "^pwm(@[0-9a-f]+)?$":
>> +    $ref: ../pwm/kontron,sl28cpld-pwm.yaml
>> +
>> +  "^watchdog(@[0-9a-f]+)?$":
>> +    $ref: ../watchdog/kontron,sl28cpld-wdt.yaml
>> +
>> +required:
>> +  - "#address-cells"
>> +  - "#size-cells"
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        sl28cpld@4a {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +            compatible = "kontron,sl28cpld-r1";
>> +            reg = <0x4a>;
> 
> Nit: Could you put the 'reg' and 'compatible' at the top please?
> 
> Same for all nodes.

Sure, I've looked at previous examples, but they are not
consistent, but it looked to me if the "#" properties are
listed first.

-michael

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

* Re: [PATCH v6 01/13] mfd: add simple regmap based I2C driver
  2020-07-28  7:42     ` Michael Walle
@ 2020-07-28  8:15       ` Lee Jones
  2020-07-28  8:27         ` Michael Walle
  0 siblings, 1 reply; 35+ messages in thread
From: Lee Jones @ 2020-07-28  8:15 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek

On Tue, 28 Jul 2020, Michael Walle wrote:

> Am 2020-07-28 09:19, schrieb Lee Jones:
> > On Sun, 26 Jul 2020, Michael Walle wrote:
> > 
> > > There are I2C devices which contain several different functions but
> > > doesn't require any special access functions. For these kind of
> > > drivers
> > > an I2C regmap should be enough.
> > > 
> > > Create an I2C driver which creates an I2C regmap and enumerates its
> > > children. If a device wants to use this as its MFD core driver, it has
> > > to add an individual compatible string. It may provide its own regmap
> > > configuration.
> > > 
> > > Subdevices can use dev_get_regmap() on the parent to get their regmap
> > > instance.
> > > 
> > > Signed-off-by: Michael Walle <michael@walle.cc>
> > > ---
> > > Changes since v5:
> > >  - removed "select MFD_CORE" in Kconfig
> > >  - removed help text in Kconfig, we assume that the users of this
> > 
> > That's the opposite of what I asked for.
> 
> What is the use to describe the symbol, if it is not user selectable?
> I'm not aware this is done anywhere in the kernel, am I missing
> something?

You mean in menuconfig?

I find 'help's helpful even outside of menuconfig.

Surely I'm not the only one who reads them 'raw'?

> > >    driver will have a "select MFD_SIMPLE_MFD_I2C". Instead added
> > >    a small description to the driver itself.
> > >  - removed "struct simple_mfd_i2c_config" and use regmap_config
> > >    directly
> > >  - changed builtin_i2c_driver() to module_i2c_driver(), added
> > >    MODULE_ boilerplate
> > >  - cleaned up the included files
> > > 
> > > Changes since v4:
> > >  - new patch. Lee, please bear with me. I didn't want to delay the
> > >    new version (where a lot of remarks on the other patches were
> > >    addressed) even more, just because we haven't figured out how
> > >    to deal with the MFD part. So for now, I've included this one.
> > > 
> > >  drivers/mfd/Kconfig          |  5 ++++
> > >  drivers/mfd/Makefile         |  1 +
> > >  drivers/mfd/simple-mfd-i2c.c | 55
> > > ++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 61 insertions(+)
> > >  create mode 100644 drivers/mfd/simple-mfd-i2c.c
> > > 
> > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > > index 33df0837ab41..c08539c7a166 100644
> > > --- a/drivers/mfd/Kconfig
> > > +++ b/drivers/mfd/Kconfig
> > > @@ -1162,6 +1162,11 @@ config MFD_SI476X_CORE
> > >  	  To compile this driver as a module, choose M here: the
> > >  	  module will be called si476x-core.
> > > 
> > > +config MFD_SIMPLE_MFD_I2C
> > > +	tristate
> > > +	depends on I2C
> > > +	select REGMAP_I2C
> > 
> > Please provide a full help.
> 
> See above.
> 
> > 
> > >  config MFD_SM501
> > >  	tristate "Silicon Motion SM501"
> > >  	depends on HAS_DMA
> > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > > index a60e5f835283..78d24a3e7c9e 100644
> > > --- a/drivers/mfd/Makefile
> > > +++ b/drivers/mfd/Makefile
> > > @@ -264,3 +264,4 @@ obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
> > >  obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
> > > 
> > >  obj-$(CONFIG_SGI_MFD_IOC3)	+= ioc3.o
> > > +obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)	+= simple-mfd-i2c.o
> > > diff --git a/drivers/mfd/simple-mfd-i2c.c
> > > b/drivers/mfd/simple-mfd-i2c.c
> > > new file mode 100644
> > > index 000000000000..45090ddad104
> > > --- /dev/null
> > > +++ b/drivers/mfd/simple-mfd-i2c.c
> > > @@ -0,0 +1,55 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * A very simple I2C MFD driver
> > 
> > Simple MFD - I2C
> 
> ok.
> 
> > > + * The driver enumerates its children and registers a common
> > > regmap. Use
> > > + * dev_get_regmap(pdev->dev.parent, NULL) in the child nodes to
> > > fetch that
> > > + * regmap instance.
> > 
> > This driver creates a single register map with the intention for it to
> > be shared by all sub-devices.  Children can use their parent's device
> > structure (dev.parent) in order reference it.
> 
> Should this be appended or should it replace my paragraph? If its the
> latter,
> the "enumeration of the children" is missing.

If you want to keep that part, try:

This driver creates a single register map with the intention for it to
be shared by all sub-devices.  Children can use their parent's device
structure (dev.parent) in order reference it.

Once the register map has been successfully initialised, any
sub-devices represented by child nodes in Device Tree will be
subsequently registered.

> > > + * In the future this driver might be extended to support also
> > > other interfaces
> > > + * like SPI etc.
> > 
> > Remove this please.
> 
> Why would you remove information about the intention of this driver? If
> someone
> looks for a place to implement its SPI/I3C/Slimbus MFD driver this might
> come
> in handy.

By all means put something similar in the commit log, but it has no
place in the driver itself.  Besides, if we were to add support for
SPI, it is likely to be a completely separate/unrelated driver.

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

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

* Re: [PATCH v6 06/13] pwm: add support for sl28cpld PWM controller
  2020-07-28  7:43   ` Uwe Kleine-König
@ 2020-07-28  8:21     ` Michael Walle
  2020-07-28  9:47       ` Uwe Kleine-König
  0 siblings, 1 reply; 35+ messages in thread
From: Michael Walle @ 2020-07-28  8:21 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Lee Jones, Thierry Reding, Wim Van Sebroeck, Shawn Guo, Li Yang,
	Thomas Gleixner, Jason Cooper, Marc Zyngier, Mark Brown,
	Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek

Hi,

Am 2020-07-28 09:43, schrieb Uwe Kleine-König:
> Hello,
> 
> just a few minor issues left:

thanks for the review.

> 
> On Sun, Jul 26, 2020 at 01:18:27AM +0200, Michael Walle wrote:
>> diff --git a/drivers/pwm/pwm-sl28cpld.c b/drivers/pwm/pwm-sl28cpld.c
>> new file mode 100644
>> index 000000000000..956fa09f3aba
>> --- /dev/null
>> +++ b/drivers/pwm/pwm-sl28cpld.c
>> @@ -0,0 +1,223 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * sl28cpld PWM driver
>> + *
>> + * Copyright (c) 2020 Michael Walle <michael@walle.cc>
>> + *
>> + * There is no public datasheet available for this PWM core. But it 
>> is easy
>> + * enough to be briefly explained. It consists of one 8-bit counter. 
>> The PWM
>> + * supports four distinct frequencies by selecting when to reset the 
>> counter.
>> + * With the prescaler setting you can select which bit of the counter 
>> is used
>> + * to reset it. This implies that the higher the frequency the less 
>> remaining
>> + * bits are available for the actual counter.
>> + *
>> + * Let cnt[7:0] be the counter, clocked at 32kHz:
>> + * +-----------+--------+--------------+-----------+
>> + * | prescaler |  reset | counter bits | frequency |
>> + * +-----------+--------+--------------+-----------+
>> + * |         0 | cnt[7] |     cnt[6:0] |     250Hz |
>> + * |         1 | cnt[6] |     cnt[5:0] |     500Hz |
>> + * |         2 | cnt[5] |     cnt[4:0] |      1kHz |
>> + * |         3 | cnt[4] |     cnt[3:0] |      2kHz |
>> + * +-----------+--------+--------------+-----------+
> 
> Very nice. I'd add a "period length" column, as this is what the PWM
> core uses.
> 
> For your convenience (and as I created that table anyhow for further
> checking of the formulas below):
> 
>  * +-----------+--------+--------------+-----------+--------+
>  * | prescaler |  reset | counter bits | frequency | period |
>  * |           |        |              |           | length |
>  * +-----------+--------+--------------+-----------+--------+
>  * |         0 | cnt[7] |     cnt[6:0] |     250Hz | 4000ns |
>  * |         1 | cnt[6] |     cnt[5:0] |     500Hz | 2000ns |
>  * |         2 | cnt[5] |     cnt[4:0] |      1kHz | 1000ns |
>  * |         3 | cnt[4] |     cnt[3:0] |      2kHz |  500ns |
>  * +-----------+--------+--------------+-----------+--------+

sure :)

> 
>> + *
>> + * Limitations:
>> + * - The hardware cannot generate a 100% duty cycle if the prescaler 
>> is 0.
>> + * - The hardware cannot atomically set the prescaler and the counter 
>> value,
>> + *   which might lead to glitches and inconsistent states if a write 
>> fails.
>> + * - The counter is not reset if you switch the prescaler which leads
>> + *   to glitches, too.
>> + * - The duty cycle will switch immediately and not after a complete 
>> cycle.
>> + * - Depending on the actual implementation, disabling the PWM might 
>> have
>> + *   side effects. For example, if the output pin is shared with a 
>> GPIO pin
>> + *   it will automatically switch back to GPIO mode.
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pwm.h>
>> +#include <linux/regmap.h>
>> +
>> +/*
>> + * PWM timer block registers.
>> + */
>> +#define SL28CPLD_PWM_CTRL			0x00
>> +#define   SL28CPLD_PWM_CTRL_ENABLE		BIT(7)
>> +#define   SL28CPLD_PWM_CTRL_PRESCALER_MASK	GENMASK(1, 0)
>> +#define SL28CPLD_PWM_CYCLE			0x01
>> +#define   SL28CPLD_PWM_CYCLE_MAX		GENMASK(6, 0)
>> +
>> +#define SL28CPLD_PWM_CLK			32000 /* 32 kHz */
>> +#define SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler)	(1 << (7 - 
>> (prescaler)))
>> +#define SL28CPLD_PWM_PERIOD(prescaler) \
>> +	(NSEC_PER_SEC / SL28CPLD_PWM_CLK * 
>> SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler))
>> +
>> +/*
>> + * We calculate the duty cycle like this:
>> + *   duty_cycle_ns = pwm_cycle_reg * max_period_ns / max_duty_cycle
>> + *
>> + * With
>> + *   max_period_ns = (1 << 7 - prescaler) / pwm_clk * NSEC_PER_SEC
>> + *   max_duty_cycle = 1 << (7 - prescaler)
> 
> If you don't need parenthesis in the max_period_ns around 7 - 
> prescaler,
> you don't need them either in the max_duty_cycle line.

mhh this should be "1 << (7 - prescaler)" in both cases. So
max_period_ns is wrong:
   max_period_ns = 1 << (7 - prescaler) / pwm_clk * NSEC_PER_SEC


>> + * this then simplifies to:
>> + *   duty_cycle_ns = pwm_cycle_reg / pwm_clk * NSEC_PER_SEC
>> + */
>> +#define SL28CPLD_PWM_TO_DUTY_CYCLE(reg) \
>> +	(NSEC_PER_SEC / SL28CPLD_PWM_CLK * (reg))
> 
> For those who copy from your driver maybe add a comment like:
> 
>  * NSEC_PER_SEC / SL28CPLD_PWM_CLK is integer here, so we're not 
> loosing
>  * precision by doing the division first.

ok.

>> +#define SL28CPLD_PWM_FROM_DUTY_CYCLE(duty_cycle) \
>> +	(DIV_ROUND_DOWN_ULL((duty_cycle), NSEC_PER_SEC / SL28CPLD_PWM_CLK))
>> +
>> +struct sl28cpld_pwm {
>> +	struct pwm_chip pwm_chip;
>> +	struct regmap *regmap;
>> +	u32 offset;
>> +};
>> +
>> +static void sl28cpld_pwm_get_state(struct pwm_chip *chip,
>> +				   struct pwm_device *pwm,
>> +				   struct pwm_state *state)
>> +{
>> +	struct sl28cpld_pwm *priv = dev_get_drvdata(chip->dev);
>> +	unsigned int reg;
>> +	int prescaler;
>> +
>> +	regmap_read(priv->regmap, priv->offset + SL28CPLD_PWM_CTRL, &reg);
> 
> Would it make sense to hide this using e.g.:
> 
> 	#define sl28cpkd_pwm_read(priv, reg, val)	regmap_read((priv)->regmap,
> (priv)->offset + (reg), val)
> 
> The line would then become:
> 
> 	sl28cpkd_pwm_read(priv, SL28CPLD_PWM_CTRL, &reg);
> 
> which is a bit prettier. Up to you to decide. If you do it, please do
> the same for write


I don't have a strong opinion on that. I can change it. Although there
will be checkpatch warning about multiple uses of the macro argument,
I'd presume.

>> +	state->enabled = reg & SL28CPLD_PWM_CTRL_ENABLE;
>> +
>> +	prescaler = FIELD_GET(SL28CPLD_PWM_CTRL_PRESCALER_MASK, reg);
>> +	state->period = SL28CPLD_PWM_PERIOD(prescaler);
>> +
>> +	regmap_read(priv->regmap, priv->offset + SL28CPLD_PWM_CYCLE, &reg);
>> +	state->duty_cycle = SL28CPLD_PWM_TO_DUTY_CYCLE(reg);
>> +	state->polarity = PWM_POLARITY_NORMAL;
>> +}
>> +
>> +static int sl28cpld_pwm_apply(struct pwm_chip *chip, struct 
>> pwm_device *pwm,
>> +			      const struct pwm_state *state)
>> +{
>> +	struct sl28cpld_pwm *priv = dev_get_drvdata(chip->dev);
>> +	unsigned int cycle, prescaler;
>> +	int ret;
>> +	u8 ctrl;
>> +
>> +	/* Polarity inversion is not supported */
>> +	if (state->polarity != PWM_POLARITY_NORMAL)
>> +		return -EINVAL;
>> +
>> +	/*
>> +	 * Calculate the prescaler. Pick the the biggest period that isn't
>> +	 * bigger than the requested period.
>> +	 */
>> +	prescaler = DIV_ROUND_UP_ULL(SL28CPLD_PWM_PERIOD(0), state->period);
>> +	prescaler = order_base_2(prescaler);
>> +
>> +	if (prescaler > field_max(SL28CPLD_PWM_CTRL_PRESCALER_MASK))
>> +		return -ERANGE;
> 
> The calculation looks right.
> Did you check the generated code? Maybe using an if or switch here is
> more effective? (optional task for bonus points :-)

I varied between this and some if/switch. This hard to read IMHO (as
was your your ilog(n+1)+1), but you could easily change the range
of the prescaler without having to change this. Also if/switch
looked ugly too *g*. I'll check again.

> 
>> +	ctrl = FIELD_PREP(SL28CPLD_PWM_CTRL_PRESCALER_MASK, prescaler);
>> +	if (state->enabled)
>> +		ctrl |= SL28CPLD_PWM_CTRL_ENABLE;
>> +
>> +	cycle = SL28CPLD_PWM_FROM_DUTY_CYCLE(state->duty_cycle);
>> +	cycle = min_t(unsigned int, cycle, 
>> SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler));
>> +
>> +	/*
>> +	 * Work around the hardware limitation. See also above. Trap 100% 
>> duty
>> +	 * cycle if the prescaler is 0. Set prescaler to 1 instead. We don't
>> +	 * care about the frequency because its "all-one" in either case.
>> +	 *
>> +	 * We don't need to check the actual prescaler setting, because only
>> +	 * if the prescaler is 0 we can have this particular value.
>> +	 */
>> +	if (cycle == SL28CPLD_PWM_MAX_DUTY_CYCLE(0)) {
>> +		ctrl &= ~SL28CPLD_PWM_CTRL_PRESCALER_MASK;
>> +		ctrl |= FIELD_PREP(SL28CPLD_PWM_CTRL_PRESCALER_MASK, 1);
>> +		cycle = SL28CPLD_PWM_MAX_DUTY_CYCLE(1);
>> +	}
>> +
>> +	ret = regmap_write(priv->regmap, priv->offset + SL28CPLD_PWM_CTRL, 
>> ctrl);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return regmap_write(priv->regmap, priv->offset + SL28CPLD_PWM_CYCLE, 
>> (u8)cycle);
> 
> This cast isn't needed, is it?

Due to the clamping, it is not, correct. I'll remove it.

>> +}
>> +
>> +static const struct pwm_ops sl28cpld_pwm_ops = {
>> +	.apply = sl28cpld_pwm_apply,
>> +	.get_state = sl28cpld_pwm_get_state,
>> +	.owner = THIS_MODULE,
>> +};
>> +
>> +static int sl28cpld_pwm_probe(struct platform_device *pdev)
>> +{
>> +	struct sl28cpld_pwm *priv;
>> +	struct pwm_chip *chip;
>> +	int ret;
>> +
>> +	if (!pdev->dev.parent)
>> +		return -ENODEV;
>> +
>> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
>> +	if (!priv->regmap)
> 
> Error message here?

This shouldn't really happen and I put it into the same category
as the two above and report no error. But I can add it.

Generally, it looked to me that more and more drivers don't
really report errors anymore, but just return with an -EWHATEVER.
So if someone can shed some light here, I'm all ears.

-michael

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

* Re: [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld
  2020-07-28  7:57     ` Michael Walle
@ 2020-07-28  8:27       ` Lee Jones
  2020-07-28  8:38         ` Michael Walle
  0 siblings, 1 reply; 35+ messages in thread
From: Lee Jones @ 2020-07-28  8:27 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek, Rob Herring

On Tue, 28 Jul 2020, Michael Walle wrote:

> Am 2020-07-28 09:24, schrieb Lee Jones:
> > On Sun, 26 Jul 2020, Michael Walle wrote:
> > 
> > > Add a device tree bindings for the board management controller found
> > > on
> > > the Kontron SMARC-sAL28 board.
> > > 
> > > Signed-off-by: Michael Walle <michael@walle.cc>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > ---
> > > Changes since v5:
> > >  - none
> > > 
> > > Changes since v4:
> > >  - fix the regex of the unit-address
> > > 
> > > Changes since v3:
> > >  - see cover letter
> > > 
> > >  .../bindings/gpio/kontron,sl28cpld-gpio.yaml  |  54 +++++++
> > >  .../hwmon/kontron,sl28cpld-hwmon.yaml         |  27 ++++
> > >  .../kontron,sl28cpld-intc.yaml                |  54 +++++++
> > >  .../bindings/mfd/kontron,sl28cpld.yaml        | 153
> > > ++++++++++++++++++
> > >  .../bindings/pwm/kontron,sl28cpld-pwm.yaml    |  35 ++++
> > >  .../watchdog/kontron,sl28cpld-wdt.yaml        |  35 ++++
> > >  6 files changed, 358 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
> > >  create mode 100644
> > > Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
> > >  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
> > >  create mode 100644
> > > Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
> > >  create mode 100644
> > > Documentation/devicetree/bindings/pwm/kontron,sl28cpld-pwm.yaml
> > >  create mode 100644
> > > Documentation/devicetree/bindings/watchdog/kontron,sl28cpld-wdt.yaml
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
> > > b/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
> > > new file mode 100644
> > > index 000000000000..9a63a158a796
> > > --- /dev/null
> > > +++
> > > b/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
> > > @@ -0,0 +1,54 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/gpio/kontron,sl28cpld-gpio.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: GPIO driver for the sl28cpld board management controller
> > > +
> > > +maintainers:
> > > +  - Michael Walle <michael@walle.cc>
> > > +
> > > +description: |
> > > +  This module is part of the sl28cpld multi-function device. For more
> > > +  details see
> > > Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
> > 
> > Paths are normally relative.
> 
> grep Documentation/ Documentation
> 
> I know there are a lot false positives (esp in the first one)..
> 
> $ grep -r "\.\./" Documentation | wc -l
> 1826
> $ grep -r "Documentation/" Documentation|wc -l
> 2862

I actually meant just for Device Tree bindings, but it does appear
that 'Documentation' is used a bunch there too.

My reasons for not liking full paths is that the intention was always
to move 'Documentation/devicetree' to a new location outside of the
kernel source tree.

[...]

> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mfd/kontron,sl28cpld.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Kontron's sl28cpld board management controller
> > 
> > "S128CPLD" ?
> 
> still not, its sl28cpld, think of a project/code name, not the product
> appended with CPLD.
> 
> > "Board Management Controller (BMC)" ?
> 
> sounds like IPMI, which I wanted to avoid.

Is there a datasheet?

> > > +maintainers:
> > > +  - Michael Walle <michael@walle.cc>
> > > +
> > > +description: |
> > > +  The board management controller may contain different IP blocks
> > > like
> > > +  watchdog, fan monitoring, PWM controller, interrupt controller
> > > and a
> > > +  GPIO controller.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: kontron,sl28cpld-r1
> > 
> > We don't usually code revision numbers in compatible strings.
> > 
> > Is there any way to pull this from the H/W?
> 
> No, unfortunately you can't. And I really want to keep that, in case
> in the future there are some backwards incompatible changes.

Rob,

I know you reviewed this already, but you can give your opinion on
this specifically please?  I know that we have pushed back on this in
the past.


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

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

* Re: [PATCH v6 01/13] mfd: add simple regmap based I2C driver
  2020-07-28  8:15       ` Lee Jones
@ 2020-07-28  8:27         ` Michael Walle
  2020-07-28  8:35           ` Lee Jones
  0 siblings, 1 reply; 35+ messages in thread
From: Michael Walle @ 2020-07-28  8:27 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek

Am 2020-07-28 10:15, schrieb Lee Jones:
> On Tue, 28 Jul 2020, Michael Walle wrote:
> 
>> Am 2020-07-28 09:19, schrieb Lee Jones:
>> > On Sun, 26 Jul 2020, Michael Walle wrote:
>> >
>> > > There are I2C devices which contain several different functions but
>> > > doesn't require any special access functions. For these kind of
>> > > drivers
>> > > an I2C regmap should be enough.
>> > >
>> > > Create an I2C driver which creates an I2C regmap and enumerates its
>> > > children. If a device wants to use this as its MFD core driver, it has
>> > > to add an individual compatible string. It may provide its own regmap
>> > > configuration.
>> > >
>> > > Subdevices can use dev_get_regmap() on the parent to get their regmap
>> > > instance.
>> > >
>> > > Signed-off-by: Michael Walle <michael@walle.cc>
>> > > ---
>> > > Changes since v5:
>> > >  - removed "select MFD_CORE" in Kconfig
>> > >  - removed help text in Kconfig, we assume that the users of this
>> >
>> > That's the opposite of what I asked for.
>> 
>> What is the use to describe the symbol, if it is not user selectable?
>> I'm not aware this is done anywhere in the kernel, am I missing
>> something?
> 
> You mean in menuconfig?
> 
> I find 'help's helpful even outside of menuconfig.
> 
> Surely I'm not the only one who reads them 'raw'?

Its already available in the header of the file. But sure, I can
copy it.

>> > >    driver will have a "select MFD_SIMPLE_MFD_I2C". Instead added
>> > >    a small description to the driver itself.
>> > >  - removed "struct simple_mfd_i2c_config" and use regmap_config
>> > >    directly
>> > >  - changed builtin_i2c_driver() to module_i2c_driver(), added
>> > >    MODULE_ boilerplate
>> > >  - cleaned up the included files
>> > >
>> > > Changes since v4:
>> > >  - new patch. Lee, please bear with me. I didn't want to delay the
>> > >    new version (where a lot of remarks on the other patches were
>> > >    addressed) even more, just because we haven't figured out how
>> > >    to deal with the MFD part. So for now, I've included this one.
>> > >
>> > >  drivers/mfd/Kconfig          |  5 ++++
>> > >  drivers/mfd/Makefile         |  1 +
>> > >  drivers/mfd/simple-mfd-i2c.c | 55
>> > > ++++++++++++++++++++++++++++++++++++
>> > >  3 files changed, 61 insertions(+)
>> > >  create mode 100644 drivers/mfd/simple-mfd-i2c.c
>> > >
>> > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> > > index 33df0837ab41..c08539c7a166 100644
>> > > --- a/drivers/mfd/Kconfig
>> > > +++ b/drivers/mfd/Kconfig
>> > > @@ -1162,6 +1162,11 @@ config MFD_SI476X_CORE
>> > >  	  To compile this driver as a module, choose M here: the
>> > >  	  module will be called si476x-core.
>> > >
>> > > +config MFD_SIMPLE_MFD_I2C
>> > > +	tristate
>> > > +	depends on I2C
>> > > +	select REGMAP_I2C
>> >
>> > Please provide a full help.
>> 
>> See above.
>> 
>> >
>> > >  config MFD_SM501
>> > >  	tristate "Silicon Motion SM501"
>> > >  	depends on HAS_DMA
>> > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> > > index a60e5f835283..78d24a3e7c9e 100644
>> > > --- a/drivers/mfd/Makefile
>> > > +++ b/drivers/mfd/Makefile
>> > > @@ -264,3 +264,4 @@ obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
>> > >  obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
>> > >
>> > >  obj-$(CONFIG_SGI_MFD_IOC3)	+= ioc3.o
>> > > +obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)	+= simple-mfd-i2c.o
>> > > diff --git a/drivers/mfd/simple-mfd-i2c.c
>> > > b/drivers/mfd/simple-mfd-i2c.c
>> > > new file mode 100644
>> > > index 000000000000..45090ddad104
>> > > --- /dev/null
>> > > +++ b/drivers/mfd/simple-mfd-i2c.c
>> > > @@ -0,0 +1,55 @@
>> > > +// SPDX-License-Identifier: GPL-2.0-only
>> > > +/*
>> > > + * A very simple I2C MFD driver
>> >
>> > Simple MFD - I2C
>> 
>> ok.
>> 
>> > > + * The driver enumerates its children and registers a common
>> > > regmap. Use
>> > > + * dev_get_regmap(pdev->dev.parent, NULL) in the child nodes to
>> > > fetch that
>> > > + * regmap instance.
>> >
>> > This driver creates a single register map with the intention for it to
>> > be shared by all sub-devices.  Children can use their parent's device
>> > structure (dev.parent) in order reference it.
>> 
>> Should this be appended or should it replace my paragraph? If its the
>> latter,
>> the "enumeration of the children" is missing.
> 
> If you want to keep that part, try:
> 
> This driver creates a single register map with the intention for it to
> be shared by all sub-devices.  Children can use their parent's device
> structure (dev.parent) in order reference it.
> 
> Once the register map has been successfully initialised, any
> sub-devices represented by child nodes in Device Tree will be
> subsequently registered.
> 
>> > > + * In the future this driver might be extended to support also
>> > > other interfaces
>> > > + * like SPI etc.
>> >
>> > Remove this please.
>> 
>> Why would you remove information about the intention of this driver? 
>> If
>> someone
>> looks for a place to implement its SPI/I3C/Slimbus MFD driver this 
>> might
>> come
>> in handy.
> 
> By all means put something similar in the commit log, but it has no
> place in the driver itself.  Besides, if we were to add support for
> SPI, it is likely to be a completely separate/unrelated driver.

Why would that be another driver? It would be 90% copy paste with
regmap_init_i2c() replaced by regmap_init_spi() and i2c_driver replaced
by spi_driver.

But I don't care. I'll remove it.

-michael

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

* Re: [PATCH v6 01/13] mfd: add simple regmap based I2C driver
  2020-07-28  8:27         ` Michael Walle
@ 2020-07-28  8:35           ` Lee Jones
  0 siblings, 0 replies; 35+ messages in thread
From: Lee Jones @ 2020-07-28  8:35 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek

On Tue, 28 Jul 2020, Michael Walle wrote:

> Am 2020-07-28 10:15, schrieb Lee Jones:
> > On Tue, 28 Jul 2020, Michael Walle wrote:
> > 
> > > Am 2020-07-28 09:19, schrieb Lee Jones:
> > > > On Sun, 26 Jul 2020, Michael Walle wrote:
> > > >
> > > > > There are I2C devices which contain several different functions but
> > > > > doesn't require any special access functions. For these kind of
> > > > > drivers
> > > > > an I2C regmap should be enough.
> > > > >
> > > > > Create an I2C driver which creates an I2C regmap and enumerates its
> > > > > children. If a device wants to use this as its MFD core driver, it has
> > > > > to add an individual compatible string. It may provide its own regmap
> > > > > configuration.
> > > > >
> > > > > Subdevices can use dev_get_regmap() on the parent to get their regmap
> > > > > instance.
> > > > >
> > > > > Signed-off-by: Michael Walle <michael@walle.cc>
> > > > > ---
> > > > > Changes since v5:
> > > > >  - removed "select MFD_CORE" in Kconfig
> > > > >  - removed help text in Kconfig, we assume that the users of this
> > > >
> > > > That's the opposite of what I asked for.
> > > 
> > > What is the use to describe the symbol, if it is not user selectable?
> > > I'm not aware this is done anywhere in the kernel, am I missing
> > > something?
> > 
> > You mean in menuconfig?
> > 
> > I find 'help's helpful even outside of menuconfig.
> > 
> > Surely I'm not the only one who reads them 'raw'?
> 
> Its already available in the header of the file. But sure, I can
> copy it.

Thanks.

[...]

> > > Why would you remove information about the intention of this driver?
> > > If
> > > someone
> > > looks for a place to implement its SPI/I3C/Slimbus MFD driver this
> > > might
> > > come
> > > in handy.
> > 
> > By all means put something similar in the commit log, but it has no
> > place in the driver itself.  Besides, if we were to add support for
> > SPI, it is likely to be a completely separate/unrelated driver.
> 
> Why would that be another driver? It would be 90% copy paste with
> regmap_init_i2c() replaced by regmap_init_spi() and i2c_driver replaced
> by spi_driver.

We'll investigate options if/when the time comes.  If 'spi_driver' and
'i2c_driver' can *sensibly* co-exist, then that is certainly an option
we can explore.

> But I don't care. I'll remove it.

Please.

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

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

* Re: [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld
  2020-07-28  8:27       ` Lee Jones
@ 2020-07-28  8:38         ` Michael Walle
  2020-07-28  8:56           ` Lee Jones
  0 siblings, 1 reply; 35+ messages in thread
From: Michael Walle @ 2020-07-28  8:38 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek, Rob Herring

Am 2020-07-28 10:27, schrieb Lee Jones:
> On Tue, 28 Jul 2020, Michael Walle wrote:
> 
>> Am 2020-07-28 09:24, schrieb Lee Jones:
>> > On Sun, 26 Jul 2020, Michael Walle wrote:
>> >
>> > > Add a device tree bindings for the board management controller found
>> > > on
>> > > the Kontron SMARC-sAL28 board.
>> > >
>> > > Signed-off-by: Michael Walle <michael@walle.cc>
>> > > Reviewed-by: Rob Herring <robh@kernel.org>
>> > > ---
>> > > Changes since v5:
>> > >  - none
>> > >
>> > > Changes since v4:
>> > >  - fix the regex of the unit-address
>> > >
>> > > Changes since v3:
>> > >  - see cover letter
>> > >
>> > >  .../bindings/gpio/kontron,sl28cpld-gpio.yaml  |  54 +++++++
>> > >  .../hwmon/kontron,sl28cpld-hwmon.yaml         |  27 ++++
>> > >  .../kontron,sl28cpld-intc.yaml                |  54 +++++++
>> > >  .../bindings/mfd/kontron,sl28cpld.yaml        | 153
>> > > ++++++++++++++++++
>> > >  .../bindings/pwm/kontron,sl28cpld-pwm.yaml    |  35 ++++
>> > >  .../watchdog/kontron,sl28cpld-wdt.yaml        |  35 ++++
>> > >  6 files changed, 358 insertions(+)
>> > >  create mode 100644
>> > > Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
>> > >  create mode 100644
>> > > Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
>> > >  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
>> > >  create mode 100644
>> > > Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
>> > >  create mode 100644
>> > > Documentation/devicetree/bindings/pwm/kontron,sl28cpld-pwm.yaml
>> > >  create mode 100644
>> > > Documentation/devicetree/bindings/watchdog/kontron,sl28cpld-wdt.yaml
>> > >
>> > > diff --git
>> > > a/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
>> > > b/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
>> > > new file mode 100644
>> > > index 000000000000..9a63a158a796
>> > > --- /dev/null
>> > > +++
>> > > b/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
>> > > @@ -0,0 +1,54 @@
>> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> > > +%YAML 1.2
>> > > +---
>> > > +$id: http://devicetree.org/schemas/gpio/kontron,sl28cpld-gpio.yaml#
>> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> > > +
>> > > +title: GPIO driver for the sl28cpld board management controller
>> > > +
>> > > +maintainers:
>> > > +  - Michael Walle <michael@walle.cc>
>> > > +
>> > > +description: |
>> > > +  This module is part of the sl28cpld multi-function device. For more
>> > > +  details see
>> > > Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
>> >
>> > Paths are normally relative.
>> 
>> grep Documentation/ Documentation
>> 
>> I know there are a lot false positives (esp in the first one)..
>> 
>> $ grep -r "\.\./" Documentation | wc -l
>> 1826
>> $ grep -r "Documentation/" Documentation|wc -l
>> 2862
> 
> I actually meant just for Device Tree bindings, but it does appear
> that 'Documentation' is used a bunch there too.
> 
> My reasons for not liking full paths is that the intention was always
> to move 'Documentation/devicetree' to a new location outside of the
> kernel source tree.

I see. I'll change that.

>> > > +%YAML 1.2
>> > > +---
>> > > +$id: http://devicetree.org/schemas/mfd/kontron,sl28cpld.yaml#
>> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> > > +
>> > > +title: Kontron's sl28cpld board management controller
>> >
>> > "S128CPLD" ?
>> 
>> still not, its sl28cpld, think of a project/code name, not the product
>> appended with CPLD.
>> 
>> > "Board Management Controller (BMC)" ?
>> 
>> sounds like IPMI, which I wanted to avoid.
> 
> Is there a datasheet?

No there isn't.

>> > > +maintainers:
>> > > +  - Michael Walle <michael@walle.cc>
>> > > +
>> > > +description: |
>> > > +  The board management controller may contain different IP blocks
>> > > like
>> > > +  watchdog, fan monitoring, PWM controller, interrupt controller
>> > > and a
>> > > +  GPIO controller.
>> > > +
>> > > +properties:
>> > > +  compatible:
>> > > +    const: kontron,sl28cpld-r1
>> >
>> > We don't usually code revision numbers in compatible strings.
>> >
>> > Is there any way to pull this from the H/W?
>> 
>> No, unfortunately you can't. And I really want to keep that, in case
>> in the future there are some backwards incompatible changes.
> 
> Rob,
> 
> I know you reviewed this already, but you can give your opinion on
> this specifically please?  I know that we have pushed back on this in
> the past.

Oh, come one. That is an arbitrary string. "sl28cpld-r1" is the first
implementation of this. A future "sl28cpld-r2" might look completely
different and might not suite the simple MFD at all. "sl28cpld" is
a made up name - as "sl28cpld-r1" is, too.

-michael

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

* Re: [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld
  2020-07-28  8:38         ` Michael Walle
@ 2020-07-28  8:56           ` Lee Jones
  2020-07-28  9:06             ` Michael Walle
  0 siblings, 1 reply; 35+ messages in thread
From: Lee Jones @ 2020-07-28  8:56 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek, Rob Herring

> > > > > +$id: http://devicetree.org/schemas/mfd/kontron,sl28cpld.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Kontron's sl28cpld board management controller
> > > >
> > > > "S128CPLD" ?
> > > 
> > > still not, its sl28cpld, think of a project/code name, not the product
> > > appended with CPLD.
> > > 
> > > > "Board Management Controller (BMC)" ?
> > > 
> > > sounds like IPMI, which I wanted to avoid.
> > 
> > Is there a datasheet?
> 
> No there isn't.

Then what are you working from?

> > > > > +maintainers:
> > > > > +  - Michael Walle <michael@walle.cc>
> > > > > +
> > > > > +description: |
> > > > > +  The board management controller may contain different IP blocks
> > > > > like
> > > > > +  watchdog, fan monitoring, PWM controller, interrupt controller
> > > > > and a
> > > > > +  GPIO controller.
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    const: kontron,sl28cpld-r1
> > > >
> > > > We don't usually code revision numbers in compatible strings.
> > > >
> > > > Is there any way to pull this from the H/W?
> > > 
> > > No, unfortunately you can't. And I really want to keep that, in case
> > > in the future there are some backwards incompatible changes.
> > 
> > Rob,
> > 
> > I know you reviewed this already, but you can give your opinion on
> > this specifically please?  I know that we have pushed back on this in
> > the past.
> 
> Oh, come one. That is an arbitrary string. "sl28cpld-r1" is the first
> implementation of this. A future "sl28cpld-r2" might look completely
> different and might not suite the simple MFD at all. "sl28cpld" is
> a made up name - as "sl28cpld-r1" is, too.

Well that sounds bogus for a start.  I guess that's one of the
problems with trying to support programmable H/W in S/W.
 
If it's okay with Rob, then I'll be okay with it.

Just to note, this has not been okay for others in the past, and this
use-case is not 'special'.

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

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

* Re: [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld
  2020-07-28  8:56           ` Lee Jones
@ 2020-07-28  9:06             ` Michael Walle
  2020-07-28  9:20               ` Lee Jones
  0 siblings, 1 reply; 35+ messages in thread
From: Michael Walle @ 2020-07-28  9:06 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek, Rob Herring

Am 2020-07-28 10:56, schrieb Lee Jones:
>> > > > > +$id: http://devicetree.org/schemas/mfd/kontron,sl28cpld.yaml#
>> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> > > > > +
>> > > > > +title: Kontron's sl28cpld board management controller
>> > > >
>> > > > "S128CPLD" ?
>> > >
>> > > still not, its sl28cpld, think of a project/code name, not the product
>> > > appended with CPLD.
>> > >
>> > > > "Board Management Controller (BMC)" ?
>> > >
>> > > sounds like IPMI, which I wanted to avoid.
>> >
>> > Is there a datasheet?
>> 
>> No there isn't.
> 
> Then what are you working from?

Ok, there is no public datasheet. If that wasn't clear before, I'm 
working
for that company that also implemented that CPLD.

>> > > > > +maintainers:
>> > > > > +  - Michael Walle <michael@walle.cc>
>> > > > > +
>> > > > > +description: |
>> > > > > +  The board management controller may contain different IP blocks
>> > > > > like
>> > > > > +  watchdog, fan monitoring, PWM controller, interrupt controller
>> > > > > and a
>> > > > > +  GPIO controller.
>> > > > > +
>> > > > > +properties:
>> > > > > +  compatible:
>> > > > > +    const: kontron,sl28cpld-r1
>> > > >
>> > > > We don't usually code revision numbers in compatible strings.
>> > > >
>> > > > Is there any way to pull this from the H/W?
>> > >
>> > > No, unfortunately you can't. And I really want to keep that, in case
>> > > in the future there are some backwards incompatible changes.
>> >
>> > Rob,
>> >
>> > I know you reviewed this already, but you can give your opinion on
>> > this specifically please?  I know that we have pushed back on this in
>> > the past.
>> 
>> Oh, come one. That is an arbitrary string. "sl28cpld-r1" is the first
>> implementation of this. A future "sl28cpld-r2" might look completely
>> different and might not suite the simple MFD at all. "sl28cpld" is
>> a made up name - as "sl28cpld-r1" is, too.
> 
> Well that sounds bogus for a start.  I guess that's one of the
> problems with trying to support programmable H/W in S/W.

What sounds bogus? That we name the implementation sl28cpld? How
is that different to like adt7411? Its just a name made up by the
vendor. So if there is a new version of the adt7411 the vendor
might name it adt7412. We name it sl28cpld-r2. So what is the
problem here?

-michael

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

* Re: [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld
  2020-07-28  9:06             ` Michael Walle
@ 2020-07-28  9:20               ` Lee Jones
  2020-07-28  9:39                 ` Michael Walle
  0 siblings, 1 reply; 35+ messages in thread
From: Lee Jones @ 2020-07-28  9:20 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek, Rob Herring

On Tue, 28 Jul 2020, Michael Walle wrote:

> Am 2020-07-28 10:56, schrieb Lee Jones:
> > > > > > > +$id: http://devicetree.org/schemas/mfd/kontron,sl28cpld.yaml#
> > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > > +
> > > > > > > +title: Kontron's sl28cpld board management controller
> > > > > >
> > > > > > "S128CPLD" ?
> > > > >
> > > > > still not, its sl28cpld, think of a project/code name, not the product
> > > > > appended with CPLD.
> > > > >
> > > > > > "Board Management Controller (BMC)" ?
> > > > >
> > > > > sounds like IPMI, which I wanted to avoid.
> > > >
> > > > Is there a datasheet?
> > > 
> > > No there isn't.
> > 
> > Then what are you working from?
> 
> Ok, there is no public datasheet. If that wasn't clear before, I'm working
> for that company that also implemented that CPLD.

No, that wasn't clear.  You said there was no datasheet.

> > > > > > > +maintainers:
> > > > > > > +  - Michael Walle <michael@walle.cc>
> > > > > > > +
> > > > > > > +description: |
> > > > > > > +  The board management controller may contain different IP blocks
> > > > > > > like
> > > > > > > +  watchdog, fan monitoring, PWM controller, interrupt controller
> > > > > > > and a
> > > > > > > +  GPIO controller.
> > > > > > > +
> > > > > > > +properties:
> > > > > > > +  compatible:
> > > > > > > +    const: kontron,sl28cpld-r1
> > > > > >
> > > > > > We don't usually code revision numbers in compatible strings.
> > > > > >
> > > > > > Is there any way to pull this from the H/W?
> > > > >
> > > > > No, unfortunately you can't. And I really want to keep that, in case
> > > > > in the future there are some backwards incompatible changes.
> > > >
> > > > Rob,
> > > >
> > > > I know you reviewed this already, but you can give your opinion on
> > > > this specifically please?  I know that we have pushed back on this in
> > > > the past.
> > > 
> > > Oh, come one. That is an arbitrary string. "sl28cpld-r1" is the first
> > > implementation of this. A future "sl28cpld-r2" might look completely
> > > different and might not suite the simple MFD at all. "sl28cpld" is
> > > a made up name - as "sl28cpld-r1" is, too.
> > 
> > Well that sounds bogus for a start.  I guess that's one of the
> > problems with trying to support programmable H/W in S/W.
> 
> What sounds bogus? That we name the implementation sl28cpld?
> How is that different to like adt7411? Its just a name made up by
> the vendor. So if there is a new version of the adt7411 the vendor 
> might name it adt7412.

Using an arbitrary string as a compatible would be bogus.

So here 'sl28cpld' is the device name, so it's not actually
arbitrary.  That's a good start.

> We name it sl28cpld-r2. So what is the problem here?

Do you though?  So 'sl28cpld-r1' is the name of the device?  The name
that is quoted from the (private) datasheet?  Because looking at the
implementation and going by the conversation, it sounds as though
you-re only adding the '-r1' piece to the compatible string for
revision identification.  Which if true, is not usually allowed and
warrants intervention by Rob.

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

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

* Re: [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld
  2020-07-28  9:20               ` Lee Jones
@ 2020-07-28  9:39                 ` Michael Walle
  0 siblings, 0 replies; 35+ messages in thread
From: Michael Walle @ 2020-07-28  9:39 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
	Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Mark Brown, Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek, Rob Herring

Am 2020-07-28 11:20, schrieb Lee Jones:
>> What sounds bogus? That we name the implementation sl28cpld?
>> How is that different to like adt7411? Its just a name made up by
>> the vendor. So if there is a new version of the adt7411 the vendor
>> might name it adt7412.
> 
> Using an arbitrary string as a compatible would be bogus.
> 
> So here 'sl28cpld' is the device name, so it's not actually
> arbitrary.  That's a good start.
> 
>> We name it sl28cpld-r2. So what is the problem here?
> 
> Do you though?  So 'sl28cpld-r1' is the name of the device?  The name
> that is quoted from the (private) datasheet?  Because looking at the
> implementation and going by the conversation, it sounds as though
> you-re only adding the '-r1' piece to the compatible string for
> revision identification.  Which if true, is not usually allowed and
> warrants intervention by Rob.

Revisions would imply backwards compatibility, correct? I'm not
aming for that. Yes, I appended that "-r1" (in the lack of any
better suffix) because I didn't want to tie the base name to the
simple MFD, just in case. And isn't that the whole purpose of
the compatible string? To connect a driver to a piece of
hardware?

But even here, I don't care anymore. I strip it again. So future
incarnations which aren't compatible with simple mfd will need
another name. So what.

-michael

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

* Re: [PATCH v6 06/13] pwm: add support for sl28cpld PWM controller
  2020-07-28  8:21     ` Michael Walle
@ 2020-07-28  9:47       ` Uwe Kleine-König
  2020-07-28 10:51         ` Lee Jones
  0 siblings, 1 reply; 35+ messages in thread
From: Uwe Kleine-König @ 2020-07-28  9:47 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Lee Jones, Thierry Reding, Wim Van Sebroeck, Shawn Guo, Li Yang,
	Thomas Gleixner, Jason Cooper, Marc Zyngier, Mark Brown,
	Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek

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

Hallo,

On Tue, Jul 28, 2020 at 10:21:22AM +0200, Michael Walle wrote:
> Am 2020-07-28 09:43, schrieb Uwe Kleine-König:
> > On Sun, Jul 26, 2020 at 01:18:27AM +0200, Michael Walle wrote:
> > > +static int sl28cpld_pwm_probe(struct platform_device *pdev)
> > > +{
> > > +	struct sl28cpld_pwm *priv;
> > > +	struct pwm_chip *chip;
> > > +	int ret;
> > > +
> > > +	if (!pdev->dev.parent)
> > > +		return -ENODEV;
> > > +
> > > +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > > +	if (!priv)
> > > +		return -ENOMEM;
> > > +
> > > +	priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> > > +	if (!priv->regmap)
> > 
> > Error message here?
> 
> This shouldn't really happen and I put it into the same category
> as the two above and report no error. But I can add it.

For kzalloc it is right to not emit an error because a failing kzalloc
is already loud on its own. I missed the first error path, that should
get a message, too.

> Generally, it looked to me that more and more drivers don't
> really report errors anymore, but just return with an -EWHATEVER.
> So if someone can shed some light here, I'm all ears.

IMHO it's wrong not to add error messages. At one point in time it will
fail and then you're happy if you don't have to add printks all over the
place first to debug that.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

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

* Re: [PATCH v6 06/13] pwm: add support for sl28cpld PWM controller
  2020-07-28  9:47       ` Uwe Kleine-König
@ 2020-07-28 10:51         ` Lee Jones
  0 siblings, 0 replies; 35+ messages in thread
From: Lee Jones @ 2020-07-28 10:51 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Michael Walle, linux-gpio, devicetree, linux-kernel, linux-hwmon,
	linux-pwm, linux-watchdog, linux-arm-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
	Thierry Reding, Wim Van Sebroeck, Shawn Guo, Li Yang,
	Thomas Gleixner, Jason Cooper, Marc Zyngier, Mark Brown,
	Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas,
	Will Deacon, Pavel Machek

On Tue, 28 Jul 2020, Uwe Kleine-König wrote:
> On Tue, Jul 28, 2020 at 10:21:22AM +0200, Michael Walle wrote:
> > Am 2020-07-28 09:43, schrieb Uwe Kleine-König:
> > > On Sun, Jul 26, 2020 at 01:18:27AM +0200, Michael Walle wrote:
> > > > +static int sl28cpld_pwm_probe(struct platform_device *pdev)
> > > > +{
> > > > +	struct sl28cpld_pwm *priv;
> > > > +	struct pwm_chip *chip;
> > > > +	int ret;
> > > > +
> > > > +	if (!pdev->dev.parent)
> > > > +		return -ENODEV;
> > > > +
> > > > +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > > > +	if (!priv)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> > > > +	if (!priv->regmap)
> > > 
> > > Error message here?
> > 
> > This shouldn't really happen and I put it into the same category
> > as the two above and report no error. But I can add it.
> 
> For kzalloc it is right to not emit an error because a failing kzalloc
> is already loud on its own. I missed the first error path, that should
> get a message, too.
> 
> > Generally, it looked to me that more and more drivers don't
> > really report errors anymore, but just return with an -EWHATEVER.
> > So if someone can shed some light here, I'm all ears.
> 
> IMHO it's wrong not to add error messages. At one point in time it will
> fail and then you're happy if you don't have to add printks all over the
> place first to debug that.

Error messages should only be omitted for -ENOMEM and if something is
already being printed out, ideally by the sub-system API.

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

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

end of thread, other threads:[~2020-07-28 10:51 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-25 23:18 [PATCH v6 00/13] Add support for Kontron sl28cpld Michael Walle
2020-07-25 23:18 ` [PATCH v6 01/13] mfd: add simple regmap based I2C driver Michael Walle
2020-07-28  7:19   ` Lee Jones
2020-07-28  7:42     ` Michael Walle
2020-07-28  8:15       ` Lee Jones
2020-07-28  8:27         ` Michael Walle
2020-07-28  8:35           ` Lee Jones
2020-07-25 23:18 ` [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld Michael Walle
2020-07-28  7:24   ` Lee Jones
2020-07-28  7:57     ` Michael Walle
2020-07-28  8:27       ` Lee Jones
2020-07-28  8:38         ` Michael Walle
2020-07-28  8:56           ` Lee Jones
2020-07-28  9:06             ` Michael Walle
2020-07-28  9:20               ` Lee Jones
2020-07-28  9:39                 ` Michael Walle
2020-07-25 23:18 ` [PATCH v6 03/13] mfd: simple-mfd-i2c: add sl28cpld support Michael Walle
2020-07-28  7:25   ` Lee Jones
2020-07-25 23:18 ` [PATCH v6 04/13] irqchip: add sl28cpld interrupt controller support Michael Walle
2020-07-25 23:18 ` [PATCH v6 05/13] watchdog: add support for sl28cpld watchdog Michael Walle
2020-07-25 23:18 ` [PATCH v6 06/13] pwm: add support for sl28cpld PWM controller Michael Walle
2020-07-27  7:30   ` Thierry Reding
2020-07-28  7:43   ` Uwe Kleine-König
2020-07-28  8:21     ` Michael Walle
2020-07-28  9:47       ` Uwe Kleine-König
2020-07-28 10:51         ` Lee Jones
2020-07-25 23:18 ` [PATCH v6 07/13] gpio: add support for the sl28cpld GPIO controller Michael Walle
2020-07-26  9:16   ` Andy Shevchenko
2020-07-25 23:18 ` [PATCH v6 08/13] hwmon: add support for the sl28cpld hardware monitoring controller Michael Walle
2020-07-26  9:18   ` Andy Shevchenko
2020-07-25 23:18 ` [PATCH v6 09/13] arm64: dts: freescale: sl28: enable sl28cpld Michael Walle
2020-07-25 23:18 ` [PATCH v6 10/13] arm64: dts: freescale: sl28: map GPIOs to input events Michael Walle
2020-07-25 23:18 ` [PATCH v6 11/13] arm64: dts: freescale: sl28: enable LED support Michael Walle
2020-07-25 23:18 ` [PATCH v6 12/13] arm64: dts: freescale: sl28: enable fan support Michael Walle
2020-07-25 23:18 ` [PATCH v6 13/13] arm64: defconfig: enable the sl28cpld board management controller Michael Walle

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).