From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C2BE3C2D0FD for ; Tue, 12 May 2020 21:59:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A327723129 for ; Tue, 12 May 2020 21:59:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1589320770; bh=4dasN4F5KmkSKWYO6iCX27SovkkmrwOz4v79E3noBoE=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=I3fpm47W8M26apFc3QcP8pZhPse1kgETSOQF+WsiL+5jE9EOantADZqIVv5S8UfIc YKF+rJ4tf4O69ruShk3zYQ273qCDS+nYGwLlIu1Ixnh7SnMGAABAZXgVxOsx0UeDtP OR2/BrBD2gfptvsUA+ltg+hU/sN8SYLPr//zz5Jw= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731538AbgELV70 (ORCPT ); Tue, 12 May 2020 17:59:26 -0400 Received: from mail.kernel.org ([198.145.29.99]:37418 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728313AbgELV70 (ORCPT ); Tue, 12 May 2020 17:59:26 -0400 Received: from mail-ot1-f49.google.com (mail-ot1-f49.google.com [209.85.210.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id EEE0320747; Tue, 12 May 2020 21:59:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1589320765; bh=4dasN4F5KmkSKWYO6iCX27SovkkmrwOz4v79E3noBoE=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=YP8cdrrciz6AD9mCNgRNuJ+8S0lvXGBwqGJDrSI3wsmZuR3wa9tIBQq17NCfG4NIB VEawFqp/icGBa36IKtrVNQyqEalQcCR4mR6xMppuuWZm7r3wGLAub9CheUDp5AcFtj x9b8IX516ah2hZFdoFmwu0PaGIRDtcOuiGZ77vHg= Received: by mail-ot1-f49.google.com with SMTP id v17so3876397ote.0; Tue, 12 May 2020 14:59:24 -0700 (PDT) X-Gm-Message-State: AGi0PubcxI9B6ddOH7OCrahvVNrApfuUU2Cs+8Q8d2gfarfzZSK5DYDn gCmz+NL6+BGEs/Kk2a9ZCoYWIp8Dlkk1xZCT+w== X-Google-Smtp-Source: APiQypL834c2wGEHb5CE5fdAX3mpa8t0DYv6uZcEwNHKazXdfkA0BR+2q+GjZ5cn3tT0/I4L/tNi+T2+uBKaDwLcHj4= X-Received: by 2002:a05:6830:4d6:: with SMTP id s22mr18608313otd.129.1589320764152; Tue, 12 May 2020 14:59:24 -0700 (PDT) MIME-Version: 1.0 References: <20200423174543.17161-1-michael@walle.cc> <20200423174543.17161-6-michael@walle.cc> <20200511211359.GB3518@bogus> In-Reply-To: From: Rob Herring Date: Tue, 12 May 2020 16:59:12 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v3 05/16] mfd: Add support for Kontron sl28cpld management controller To: Michael Walle Cc: Andy Shevchenko , "open list:GPIO SUBSYSTEM" , devicetree@vger.kernel.org, "linux-kernel@vger.kernel.org" , Linux HWMON List , Linux PWM List , LINUX-WATCHDOG , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" , Linus Walleij , Bartosz Golaszewski , Jean Delvare , Guenter Roeck , Lee Jones , Thierry Reding , =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , Wim Van Sebroeck , Shawn Guo , Li Yang , Thomas Gleixner , Jason Cooper , Marc Zyngier , Mark Brown , Greg Kroah-Hartman Content-Type: text/plain; charset="UTF-8" Sender: linux-hwmon-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-hwmon@vger.kernel.org On Mon, May 11, 2020 at 4:45 PM Michael Walle wrote: > > Am 2020-05-11 23:13, schrieb Rob Herring: > > On Thu, Apr 23, 2020 at 07:45:32PM +0200, Michael Walle wrote: > >> This patch adds core support for the board management controller found > >> on the SMARC-sAL28 board. It consists of the following functions: > >> - watchdog > >> - GPIO controller > >> - PWM controller > >> - fan sensor > >> - interrupt controller > >> > >> At the moment, this controller is used on the Kontron SMARC-sAL28 > >> board. > >> > >> Please note that the MFD driver is defined as bool in the Kconfig > >> because the next patch will add interrupt support. > >> > >> Signed-off-by: Michael Walle > >> --- > >> drivers/mfd/Kconfig | 19 +++++ > >> drivers/mfd/Makefile | 2 + > >> drivers/mfd/sl28cpld.c | 153 > >> +++++++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 174 insertions(+) > >> create mode 100644 drivers/mfd/sl28cpld.c > >> > >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > >> index 0a59249198d3..be0c8d93c526 100644 > >> --- a/drivers/mfd/Kconfig > >> +++ b/drivers/mfd/Kconfig > >> @@ -2060,5 +2060,24 @@ config SGI_MFD_IOC3 > >> If you have an SGI Origin, Octane, or a PCI IOC3 card, > >> then say Y. Otherwise say N. > >> > >> +config MFD_SL28CPLD > >> + bool "Kontron sl28 core driver" > >> + depends on I2C=y > >> + depends on OF > >> + select REGMAP_I2C > >> + select MFD_CORE > >> + help > >> + This option enables support for the board management controller > >> + found on the Kontron sl28 CPLD. You have to select individual > >> + functions, such as watchdog, GPIO, etc, under the corresponding > >> menus > >> + in order to enable them. > >> + > >> + Currently supported boards are: > >> + > >> + Kontron SMARC-sAL28 > >> + > >> + To compile this driver as a module, choose M here: the module will > >> be > >> + called sl28cpld. > >> + > >> endmenu > >> endif > >> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > >> index f935d10cbf0f..9bc38863b9c7 100644 > >> --- a/drivers/mfd/Makefile > >> +++ b/drivers/mfd/Makefile > >> @@ -259,3 +259,5 @@ obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o > >> obj-$(CONFIG_MFD_STMFX) += stmfx.o > >> > >> obj-$(CONFIG_SGI_MFD_IOC3) += ioc3.o > >> + > >> +obj-$(CONFIG_MFD_SL28CPLD) += sl28cpld.o > >> diff --git a/drivers/mfd/sl28cpld.c b/drivers/mfd/sl28cpld.c > >> new file mode 100644 > >> index 000000000000..1e5860cc7ffc > >> --- /dev/null > >> +++ b/drivers/mfd/sl28cpld.c > >> @@ -0,0 +1,153 @@ > >> +// SPDX-License-Identifier: GPL-2.0-only > >> +/* > >> + * MFD core for the sl28cpld. > >> + * > >> + * Copyright 2019 Kontron Europe GmbH > >> + */ > >> + > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> + > >> +#define SL28CPLD_VERSION 0x03 > >> +#define SL28CPLD_WATCHDOG_BASE 0x04 > >> +#define SL28CPLD_HWMON_FAN_BASE 0x0b > >> +#define SL28CPLD_PWM0_BASE 0x0c > >> +#define SL28CPLD_PWM1_BASE 0x0e > >> +#define SL28CPLD_GPIO0_BASE 0x10 > >> +#define SL28CPLD_GPIO1_BASE 0x15 > >> +#define SL28CPLD_GPO_BASE 0x1a > >> +#define SL28CPLD_GPI_BASE 0x1b > >> +#define SL28CPLD_INTC_BASE 0x1c > > > > If you want to use 'reg' in the binding, these are the numbers you > > should be using rather than making up numbering! > > My motivation is that I don't want to hardcode the internal addresses > of the management controller in the device tree. For example if they > will move around with a later update of the controller, so a driver can > be compatible with both the old and the new version. If they are in the > device tree, only one register layout is possible. I don't understand, if the addresses change, then the above defines have to change. So your driver is only compatible with 1 version. If you change the CPLD, then that's a h/w change and your h/w description (DT) should change. That can either be the compatible string changing and updating the driver with new match data such as register offsets or all the differences are in DT and there's no kernel change. > > However, I still don't think you need any child nodes. All the data in > > the DT binding is right here in the driver already. There's no > > advantage > > to putting child nodes in DT, because this driver still has to be > > updated if you add more nodes. > > But then any phandle will reference the mfd device. And for example > there > are two different interrupt controllers, that is the INTC and the > GPIO[01], > which will then be combined into one device tree node, right? You either have to add a cell for 'bank' or divide the 1st cell into a bank and index. Both have been done before. To go the other direction, AIUI you shouldn't need OF_MFD_CELL_REG entries if you have the child devices in DT. Pick one way or the other. It's ultimately a judgement call. For a one-off device, sub devices in DT doesn't really buy you anything. If you have sub-blocks showing up multiple devices, then sub devices makes sense. If there's only 2-3 combinations, then it's a toss up. Rob From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rob Herring Subject: Re: [PATCH v3 05/16] mfd: Add support for Kontron sl28cpld management controller Date: Tue, 12 May 2020 16:59:12 -0500 Message-ID: References: <20200423174543.17161-1-michael@walle.cc> <20200423174543.17161-6-michael@walle.cc> <20200511211359.GB3518@bogus> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: Sender: linux-gpio-owner@vger.kernel.org To: Michael Walle Cc: Andy Shevchenko , "open list:GPIO SUBSYSTEM" , devicetree@vger.kernel.org, "linux-kernel@vger.kernel.org" , Linux HWMON List , Linux PWM List , LINUX-WATCHDOG , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" , Linus Walleij , Bartosz Golaszewski , Jean Delvare , Guenter Roeck , Lee Jones , Thierry Reding , =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , Wim List-Id: linux-pwm@vger.kernel.org On Mon, May 11, 2020 at 4:45 PM Michael Walle wrote: > > Am 2020-05-11 23:13, schrieb Rob Herring: > > On Thu, Apr 23, 2020 at 07:45:32PM +0200, Michael Walle wrote: > >> This patch adds core support for the board management controller found > >> on the SMARC-sAL28 board. It consists of the following functions: > >> - watchdog > >> - GPIO controller > >> - PWM controller > >> - fan sensor > >> - interrupt controller > >> > >> At the moment, this controller is used on the Kontron SMARC-sAL28 > >> board. > >> > >> Please note that the MFD driver is defined as bool in the Kconfig > >> because the next patch will add interrupt support. > >> > >> Signed-off-by: Michael Walle > >> --- > >> drivers/mfd/Kconfig | 19 +++++ > >> drivers/mfd/Makefile | 2 + > >> drivers/mfd/sl28cpld.c | 153 > >> +++++++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 174 insertions(+) > >> create mode 100644 drivers/mfd/sl28cpld.c > >> > >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > >> index 0a59249198d3..be0c8d93c526 100644 > >> --- a/drivers/mfd/Kconfig > >> +++ b/drivers/mfd/Kconfig > >> @@ -2060,5 +2060,24 @@ config SGI_MFD_IOC3 > >> If you have an SGI Origin, Octane, or a PCI IOC3 card, > >> then say Y. Otherwise say N. > >> > >> +config MFD_SL28CPLD > >> + bool "Kontron sl28 core driver" > >> + depends on I2C=y > >> + depends on OF > >> + select REGMAP_I2C > >> + select MFD_CORE > >> + help > >> + This option enables support for the board management controller > >> + found on the Kontron sl28 CPLD. You have to select individual > >> + functions, such as watchdog, GPIO, etc, under the corresponding > >> menus > >> + in order to enable them. > >> + > >> + Currently supported boards are: > >> + > >> + Kontron SMARC-sAL28 > >> + > >> + To compile this driver as a module, choose M here: the module will > >> be > >> + called sl28cpld. > >> + > >> endmenu > >> endif > >> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > >> index f935d10cbf0f..9bc38863b9c7 100644 > >> --- a/drivers/mfd/Makefile > >> +++ b/drivers/mfd/Makefile > >> @@ -259,3 +259,5 @@ obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o > >> obj-$(CONFIG_MFD_STMFX) += stmfx.o > >> > >> obj-$(CONFIG_SGI_MFD_IOC3) += ioc3.o > >> + > >> +obj-$(CONFIG_MFD_SL28CPLD) += sl28cpld.o > >> diff --git a/drivers/mfd/sl28cpld.c b/drivers/mfd/sl28cpld.c > >> new file mode 100644 > >> index 000000000000..1e5860cc7ffc > >> --- /dev/null > >> +++ b/drivers/mfd/sl28cpld.c > >> @@ -0,0 +1,153 @@ > >> +// SPDX-License-Identifier: GPL-2.0-only > >> +/* > >> + * MFD core for the sl28cpld. > >> + * > >> + * Copyright 2019 Kontron Europe GmbH > >> + */ > >> + > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> + > >> +#define SL28CPLD_VERSION 0x03 > >> +#define SL28CPLD_WATCHDOG_BASE 0x04 > >> +#define SL28CPLD_HWMON_FAN_BASE 0x0b > >> +#define SL28CPLD_PWM0_BASE 0x0c > >> +#define SL28CPLD_PWM1_BASE 0x0e > >> +#define SL28CPLD_GPIO0_BASE 0x10 > >> +#define SL28CPLD_GPIO1_BASE 0x15 > >> +#define SL28CPLD_GPO_BASE 0x1a > >> +#define SL28CPLD_GPI_BASE 0x1b > >> +#define SL28CPLD_INTC_BASE 0x1c > > > > If you want to use 'reg' in the binding, these are the numbers you > > should be using rather than making up numbering! > > My motivation is that I don't want to hardcode the internal addresses > of the management controller in the device tree. For example if they > will move around with a later update of the controller, so a driver can > be compatible with both the old and the new version. If they are in the > device tree, only one register layout is possible. I don't understand, if the addresses change, then the above defines have to change. So your driver is only compatible with 1 version. If you change the CPLD, then that's a h/w change and your h/w description (DT) should change. That can either be the compatible string changing and updating the driver with new match data such as register offsets or all the differences are in DT and there's no kernel change. > > However, I still don't think you need any child nodes. All the data in > > the DT binding is right here in the driver already. There's no > > advantage > > to putting child nodes in DT, because this driver still has to be > > updated if you add more nodes. > > But then any phandle will reference the mfd device. And for example > there > are two different interrupt controllers, that is the INTC and the > GPIO[01], > which will then be combined into one device tree node, right? You either have to add a cell for 'bank' or divide the 1st cell into a bank and index. Both have been done before. To go the other direction, AIUI you shouldn't need OF_MFD_CELL_REG entries if you have the child devices in DT. Pick one way or the other. It's ultimately a judgement call. For a one-off device, sub devices in DT doesn't really buy you anything. If you have sub-blocks showing up multiple devices, then sub devices makes sense. If there's only 2-3 combinations, then it's a toss up. Rob From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 19B4CC2D0F8 for ; Tue, 12 May 2020 21:59:36 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DA60620747 for ; Tue, 12 May 2020 21:59:35 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="NJV/TLwK"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="YP8cdrrc" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DA60620747 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=EFPvY4GVtFN7uCtQZr9ajk2Qiv/zfdr44VFhYxI3/n4=; b=NJV/TLwKDSnTQs wOXIPzWT6O8JlX1VrYfuQqjnEIxGpDflmvfx3WnDdu+BHerbSBTowBUuKbSrqiQh2wXb+x/+5Or1j pB4tw70VxoTxQ6E7EucRyDIUFWWqjYA6UQw7uf5MqCt8A6CtAznhquP19XcPCckblL6h+pRgfOVq1 TomcgzdwmfYJjBdzEXnOQ3Dq+3LtuVf3s3ZM8dhuyL6eDKOlhk9i58cPFAPyuPXLkMBSSy8OMnVNN Okitb6GkA4KxuUeFQD1NcTiB7oGSovt+wkEtpSbDr/r+oljZVeHvRTYkB/fogbG7XKwfkMfMS7SyH bWWSLNoy2o45nUQHDWtA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jYcvx-0003AR-22; Tue, 12 May 2020 21:59:29 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jYcvt-00039y-Po for linux-arm-kernel@lists.infradead.org; Tue, 12 May 2020 21:59:27 +0000 Received: from mail-ot1-f41.google.com (mail-ot1-f41.google.com [209.85.210.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 0C8DF2492A for ; Tue, 12 May 2020 21:59:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1589320765; bh=4dasN4F5KmkSKWYO6iCX27SovkkmrwOz4v79E3noBoE=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=YP8cdrrciz6AD9mCNgRNuJ+8S0lvXGBwqGJDrSI3wsmZuR3wa9tIBQq17NCfG4NIB VEawFqp/icGBa36IKtrVNQyqEalQcCR4mR6xMppuuWZm7r3wGLAub9CheUDp5AcFtj x9b8IX516ah2hZFdoFmwu0PaGIRDtcOuiGZ77vHg= Received: by mail-ot1-f41.google.com with SMTP id t3so11821508otp.3 for ; Tue, 12 May 2020 14:59:25 -0700 (PDT) X-Gm-Message-State: AGi0PuZ94DfNq7ZgYZ3SygljSOy9V9NfBghsUKCu/1uz7wHn/e1aTjaY egy7dbvhLotB8ekUKt8Bcc3YYtmv8Yb+01tw9w== X-Google-Smtp-Source: APiQypL834c2wGEHb5CE5fdAX3mpa8t0DYv6uZcEwNHKazXdfkA0BR+2q+GjZ5cn3tT0/I4L/tNi+T2+uBKaDwLcHj4= X-Received: by 2002:a05:6830:4d6:: with SMTP id s22mr18608313otd.129.1589320764152; Tue, 12 May 2020 14:59:24 -0700 (PDT) MIME-Version: 1.0 References: <20200423174543.17161-1-michael@walle.cc> <20200423174543.17161-6-michael@walle.cc> <20200511211359.GB3518@bogus> In-Reply-To: From: Rob Herring Date: Tue, 12 May 2020 16:59:12 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v3 05/16] mfd: Add support for Kontron sl28cpld management controller To: Michael Walle X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200512_145925_877037_97913A30 X-CRM114-Status: GOOD ( 34.78 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, Linus Walleij , Thierry Reding , Lee Jones , Jason Cooper , Andy Shevchenko , Marc Zyngier , Bartosz Golaszewski , =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , Guenter Roeck , Linux PWM List , Jean Delvare , LINUX-WATCHDOG , "open list:GPIO SUBSYSTEM" , Mark Brown , Thomas Gleixner , Wim Van Sebroeck , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" , Linux HWMON List , Greg Kroah-Hartman , "linux-kernel@vger.kernel.org" , Li Yang , Shawn Guo Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, May 11, 2020 at 4:45 PM Michael Walle wrote: > > Am 2020-05-11 23:13, schrieb Rob Herring: > > On Thu, Apr 23, 2020 at 07:45:32PM +0200, Michael Walle wrote: > >> This patch adds core support for the board management controller found > >> on the SMARC-sAL28 board. It consists of the following functions: > >> - watchdog > >> - GPIO controller > >> - PWM controller > >> - fan sensor > >> - interrupt controller > >> > >> At the moment, this controller is used on the Kontron SMARC-sAL28 > >> board. > >> > >> Please note that the MFD driver is defined as bool in the Kconfig > >> because the next patch will add interrupt support. > >> > >> Signed-off-by: Michael Walle > >> --- > >> drivers/mfd/Kconfig | 19 +++++ > >> drivers/mfd/Makefile | 2 + > >> drivers/mfd/sl28cpld.c | 153 > >> +++++++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 174 insertions(+) > >> create mode 100644 drivers/mfd/sl28cpld.c > >> > >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > >> index 0a59249198d3..be0c8d93c526 100644 > >> --- a/drivers/mfd/Kconfig > >> +++ b/drivers/mfd/Kconfig > >> @@ -2060,5 +2060,24 @@ config SGI_MFD_IOC3 > >> If you have an SGI Origin, Octane, or a PCI IOC3 card, > >> then say Y. Otherwise say N. > >> > >> +config MFD_SL28CPLD > >> + bool "Kontron sl28 core driver" > >> + depends on I2C=y > >> + depends on OF > >> + select REGMAP_I2C > >> + select MFD_CORE > >> + help > >> + This option enables support for the board management controller > >> + found on the Kontron sl28 CPLD. You have to select individual > >> + functions, such as watchdog, GPIO, etc, under the corresponding > >> menus > >> + in order to enable them. > >> + > >> + Currently supported boards are: > >> + > >> + Kontron SMARC-sAL28 > >> + > >> + To compile this driver as a module, choose M here: the module will > >> be > >> + called sl28cpld. > >> + > >> endmenu > >> endif > >> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > >> index f935d10cbf0f..9bc38863b9c7 100644 > >> --- a/drivers/mfd/Makefile > >> +++ b/drivers/mfd/Makefile > >> @@ -259,3 +259,5 @@ obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o > >> obj-$(CONFIG_MFD_STMFX) += stmfx.o > >> > >> obj-$(CONFIG_SGI_MFD_IOC3) += ioc3.o > >> + > >> +obj-$(CONFIG_MFD_SL28CPLD) += sl28cpld.o > >> diff --git a/drivers/mfd/sl28cpld.c b/drivers/mfd/sl28cpld.c > >> new file mode 100644 > >> index 000000000000..1e5860cc7ffc > >> --- /dev/null > >> +++ b/drivers/mfd/sl28cpld.c > >> @@ -0,0 +1,153 @@ > >> +// SPDX-License-Identifier: GPL-2.0-only > >> +/* > >> + * MFD core for the sl28cpld. > >> + * > >> + * Copyright 2019 Kontron Europe GmbH > >> + */ > >> + > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> + > >> +#define SL28CPLD_VERSION 0x03 > >> +#define SL28CPLD_WATCHDOG_BASE 0x04 > >> +#define SL28CPLD_HWMON_FAN_BASE 0x0b > >> +#define SL28CPLD_PWM0_BASE 0x0c > >> +#define SL28CPLD_PWM1_BASE 0x0e > >> +#define SL28CPLD_GPIO0_BASE 0x10 > >> +#define SL28CPLD_GPIO1_BASE 0x15 > >> +#define SL28CPLD_GPO_BASE 0x1a > >> +#define SL28CPLD_GPI_BASE 0x1b > >> +#define SL28CPLD_INTC_BASE 0x1c > > > > If you want to use 'reg' in the binding, these are the numbers you > > should be using rather than making up numbering! > > My motivation is that I don't want to hardcode the internal addresses > of the management controller in the device tree. For example if they > will move around with a later update of the controller, so a driver can > be compatible with both the old and the new version. If they are in the > device tree, only one register layout is possible. I don't understand, if the addresses change, then the above defines have to change. So your driver is only compatible with 1 version. If you change the CPLD, then that's a h/w change and your h/w description (DT) should change. That can either be the compatible string changing and updating the driver with new match data such as register offsets or all the differences are in DT and there's no kernel change. > > However, I still don't think you need any child nodes. All the data in > > the DT binding is right here in the driver already. There's no > > advantage > > to putting child nodes in DT, because this driver still has to be > > updated if you add more nodes. > > But then any phandle will reference the mfd device. And for example > there > are two different interrupt controllers, that is the INTC and the > GPIO[01], > which will then be combined into one device tree node, right? You either have to add a cell for 'bank' or divide the 1st cell into a bank and index. Both have been done before. To go the other direction, AIUI you shouldn't need OF_MFD_CELL_REG entries if you have the child devices in DT. Pick one way or the other. It's ultimately a judgement call. For a one-off device, sub devices in DT doesn't really buy you anything. If you have sub-blocks showing up multiple devices, then sub devices makes sense. If there's only 2-3 combinations, then it's a toss up. Rob _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel