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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6E977C433FE for ; Fri, 26 Nov 2021 13:30:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346159AbhKZNeF (ORCPT ); Fri, 26 Nov 2021 08:34:05 -0500 Received: from david.siemens.de ([192.35.17.14]:41811 "EHLO david.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1349533AbhKZNcE (ORCPT ); Fri, 26 Nov 2021 08:32:04 -0500 Received: from mail1.sbs.de (mail1.sbs.de [192.129.41.35]) by david.siemens.de (8.15.2/8.15.2) with ESMTPS id 1AQDSSOu018897 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 26 Nov 2021 14:28:28 +0100 Received: from md1za8fc.ad001.siemens.net ([139.22.47.90]) by mail1.sbs.de (8.15.2/8.15.2) with ESMTP id 1AQDSSej007519; Fri, 26 Nov 2021 14:28:28 +0100 Date: Fri, 26 Nov 2021 14:28:27 +0100 From: Henning Schild To: Andy Shevchenko Cc: Linux Kernel Mailing List , Linux LED Subsystem , Platform Driver , linux-watchdog@vger.kernel.org, Srikanth Krishnakar , Jan Kiszka , Gerd Haeussler , Guenter Roeck , Wim Van Sebroeck , Mark Gross , Hans de Goede , Pavel Machek , Enrico Weigelt Subject: Re: [PATCH v3 2/4] leds: simatic-ipc-leds: add new driver for Siemens Industial PCs Message-ID: <20211126142827.78d2348d@md1za8fc.ad001.siemens.net> In-Reply-To: References: <20210329174928.18816-1-henning.schild@siemens.com> <20210329174928.18816-3-henning.schild@siemens.com> X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: platform-driver-x86@vger.kernel.org Am Tue, 30 Mar 2021 14:04:35 +0300 schrieb Andy Shevchenko : > On Mon, Mar 29, 2021 at 8:59 PM Henning Schild > wrote: > > > > This driver adds initial support for several devices from Siemens. > > It is based on a platform driver introduced in an earlier commit. > > ... > > > +#define SIMATIC_IPC_LED_PORT_BASE 0x404E > > > +static struct simatic_ipc_led simatic_ipc_leds_io[] = { > > + {1 << 15, "green:" LED_FUNCTION_STATUS "-1" }, > > + {1 << 7, "yellow:" LED_FUNCTION_STATUS "-1" }, > > + {1 << 14, "red:" LED_FUNCTION_STATUS "-2" }, > > + {1 << 6, "yellow:" LED_FUNCTION_STATUS "-2" }, > > + {1 << 13, "red:" LED_FUNCTION_STATUS "-3" }, > > + {1 << 5, "yellow:" LED_FUNCTION_STATUS "-3" }, > > + { } > > +}; > > > +static struct simatic_ipc_led simatic_ipc_leds_mem[] = { > > + {0x500 + 0x1A0, "red:" LED_FUNCTION_STATUS "-1"}, > > + {0x500 + 0x1A8, "green:" LED_FUNCTION_STATUS "-1"}, > > + {0x500 + 0x1C8, "red:" LED_FUNCTION_STATUS "-2"}, > > + {0x500 + 0x1D0, "green:" LED_FUNCTION_STATUS "-2"}, > > + {0x500 + 0x1E0, "red:" LED_FUNCTION_STATUS "-3"}, > > + {0x500 + 0x198, "green:" LED_FUNCTION_STATUS "-3"}, > > + { } > > +}; > > It seems to me like poking GPIO controller registers directly. This > is not good. The question still remains: Can we simply register a > GPIO (pin control) driver and use an LED GPIO driver with an > additional board file that instantiates it? The short answer for v4 will be "No we can not!". The pinctrl drivers do not currently probe on any of the devices and attempts to fix that have failed or gut stuck. I tried to help out where i could and waited for a long time. Now my take is to turn the order around. We go in like that and will happily switch to pinctrl if that ever comes up on the machines. Meaning P2SB series on top of this, no more delays please. We do use request_region so have a mutex in place. Meaning we really only touch GPIO while pinctrl does not! I see no issue here, waited for a long time and now expect to be allowed to get merged first. Henning