Date:   Thu, 2 Feb 2023 20:57:04 +0100
From:   Henning Schild <henning.schild@siemens.com>
To:     Lee Jones <lee@kernel.org>
Cc:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Pavel Machek <pavel@ucw.cz>, linux-leds@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Andy Shevchenko <andy.shevchenko@gmail.com>
Subject: Re: [PATCH v4] leds: simatic-ipc-leds-gpio: make sure we have the
 GPIO providing driver
Message-ID: <20230202205704.12a5fbff@md1za8fc.ad001.siemens.net>
In-Reply-To: <Y8mv8PzL1UsP9gNh@google.com>
References: <20221007153323.1326-1-henning.schild@siemens.com>
        <Y8mv8PzL1UsP9gNh@google.com>
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?xME9zAGaYoQGWuzE21S3RHAQTmgWnev6UiB+A7jol94bJoIw20JXVenGNpZq?=
 =?us-ascii?Q?islHdzOlf/X76M/PU0e4IKpIxaZK/IGGtcTJ4nZJCBtOC1Uz3vnx2CxMUmNt?=
 =?us-ascii?Q?fq2ljWUn4eCGgSTPrJ3CodMt08AAhiFO9Ih9ehBY5e0f/7tv3cemDuXdEuDC?=
 =?us-ascii?Q?TTzwgyEq8QfioIzqC6d5n6fUs1+FlgsgyFSyPhjq9GWFByyJqpazJ89R0UAB?=
 =?us-ascii?Q?AqJB3cX8iqgJf5jDbqfezPrOKhui70CELRqzFwcvxmlEYFd1/vIaA1QZZ2eS?=
 =?us-ascii?Q?k4nEg0jBITsMtH6PvVna9bA5ikE8BEy7cEGgfSaY3TsSJRIJwxSb2XIgSDY+?=
 =?us-ascii?Q?JJtPL4QVY2CLxVBz05QpJOLR43plkyThucEUtzsLfeEOXVEyfuCXD/jck/n0?=
 =?us-ascii?Q?IngopLJFrCJ6izqmz2eCCKKx8YsDTa8SOczLxaFqLq2oHdts+2RbNrAtEiqs?=
 =?us-ascii?Q?kbaM0gpSdxNoJ+2L67mppaBpFWQmlZ+yy/LJJS3AeXLRbYN63dDuHjbEkytt?=
 =?us-ascii?Q?B2sCCgPdGoceSPCES6G6+a/kLOZvm+aVY2PkUpLS/g5EIADFVTkcEBXyR/m5?=
 =?us-ascii?Q?kJa+9QJ/LQYSLjFDI2xaOXAp7OH5CK/sU0gj+5jQypiLA+urpdriz8isu3/i?=
 =?us-ascii?Q?MWEmAPHi+8IU1ocbNYfVviXjhaJpTzRxLWg9ZE/SC7Iyktry4V2jEkswCoPb?=
 =?us-ascii?Q?VfEvhmhR/zLeaMuuLZMzM4jjDqb4DMf+A2Zg5vmO3QqjKdzlpjF2YRHgkwNq?=
 =?us-ascii?Q?516vWrMs8x/PWXhJXiGd1qYoef7scEDc4Cn5zVIV30hKhU2S1icyiYwlCoGY?=
 =?us-ascii?Q?y+dxhjs6eH5fQBnkJDvg39Eto1UnUl/VVvzjBHjIRpDVCF0tWKPxr1b0nL9B?=
 =?us-ascii?Q?NYi12w7pUZqEz3aDoc7fmF0LU4JKUGVDUYclQuO++Xya9hLt8g7MuhKHqFxr?=
 =?us-ascii?Q?b+fwF94QsOpaUZ40QNOAg4QpgvYj0WnVG+wMvfMQTm794jczv0KU3PPnkV1w?=
 =?us-ascii?Q?b4yZCQvNvbNw6wC4RlmpXwq/1Uk8MoX4s/vUAIVOIUfFknwyBhVPjMtkfiBF?=
 =?us-ascii?Q?4PHDRFQ05vFtPH1aNOV61Hm+bkQUTqWm6/IrwNfDf5WTTsv1sSKYNUXkLQ6y?=
 =?us-ascii?Q?+qf31dcA+0+onm1gll6IO5tYcN7oukw7fOP+dIn+9rkJM5MMrtkX1Ka8UfK7?=
 =?us-ascii?Q?d25xcYOE+dfdaBHAJZd8VvBQNjeaGweJY0Klby9m2JFAmHKOwS6LwaRRvG14?=
 =?us-ascii?Q?U2HFYbGUKbGpodtrptbKXffARXQRI+VigIiEtQ1TSW+GJFrF06y96kyLyqrN?=
 =?us-ascii?Q?pZvqH6TJZIeP8RSbi7+FFzjTqUQUf/OQQqCE9J6b3vYOPPne4g6HBQAw9Usa?=
 =?us-ascii?Q?A7RPfTFAdMx3fIXzdy+Mf0Z5mQFE1yZcYGIKnZ8dQFI/d3vCeCL6f0YDFywx?=
 =?us-ascii?Q?a6KMPQQwfeHdqhd2kdWWlOSqhhAnFwFcDWqqK6YwR3EmbppFLuQZLsUV809F?=
 =?us-ascii?Q?KJH+hUrmjqD6azrZ7yN5yAv0IxpcLBdifqt3pbRAW1bdo+ZQXhVsfu93isyJ?=
 =?us-ascii?Q?2ZgZu12t+cJn63MqEJJYSSBzloBVdbwOjuv4XwXVNxK8/dm+tsdS0GASTekF?=
 =?us-ascii?Q?e5pXy9u9C1eJah1Y8TEy3hPT5xMNKXm9MEoO/axCne1TCjnoGqKdSdtTnZ0W?=
 =?us-ascii?Q?vYEYyA=3D=3D?=
X-OriginatorOrg: siemens.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8f522b03-13f6-44a1-1740-08db0557aa00
X-MS-Exchange-CrossTenant-AuthSource: PA4PR10MB5780.EURPRD10.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Feb 2023 19:57:07.9545
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 38ae3bcd-9579-4fd4-adda-b42e1495d55a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: wm6wFUuDqaHM9BWn3EN43JgOzGaWLvpuS6UJuvAdIt1kwEBrg+7vRz9sdR/tQX96ZcHQr/6ki6DPdnXneyJwtF1rQswE/tr/CWveNXg/Sc4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR10MB7523
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am Thu, 19 Jan 2023 21:02:40 +0000
schrieb Lee Jones <lee@kernel.org>:

> On Fri, 07 Oct 2022, Henning Schild wrote:
> 
> > If we register a "leds-gpio" platform device for GPIO pins that do
> > not exist we get a -EPROBE_DEFER and the probe will be tried again
> > later. If there is no driver to provide that pin we will poll
> > forever and also create a lot of log messages.
> > 
> > So check if that GPIO driver is configured, if so it will come up
> > eventually. If not, we exit our probe function early and do not even
> > bother registering the "leds-gpio". This method was chosen over
> > "Kconfig depends" since this way we can add support for more
> > devices and GPIO backends more easily without "depends":ing on all
> > GPIO backends.
> > 
> > Fixes: a6c80bec3c93 ("leds: simatic-ipc-leds-gpio: Add GPIO version
> > of Siemens driver") Reviewed-by: Andy Shevchenko
> > <andy.shevchenko@gmail.com> Signed-off-by: Henning Schild
> > <henning.schild@siemens.com> ---
> >  drivers/leds/simple/simatic-ipc-leds-gpio.c | 2 ++
> >  1 file changed, 2 insertions(+)  
> 
> FYI: I'm going to try my best not to take another one like this.

You will not have to. I now understood how to improve on that as i am
adding more variants needing more gpio controller drivers.

> Please try to improve the whole situation for you next submission.
> 
> Applied, thanks.

I hope this is still in the branches for a merge. It should be applied.
It does fix a problem but using a wrong pattern, but a pattern that is
already in use.

So this will fix 6.1 and above in the short term.

In the long term i will restructure to individual drivers which have a
clear dependency chain in Kconfig. I will use inheritance to arrive at
minimal code duplication and will use Kconfig switch default
inheritance to ease configuration.

Such restructuring patches will have to be written first, but they will
come. Either stand-alone or together with the next machine.

regards,
Henning