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 B15A2C433F5 for ; Mon, 23 May 2022 15:12:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237626AbiEWPME (ORCPT ); Mon, 23 May 2022 11:12:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57716 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237587AbiEWPL7 (ORCPT ); Mon, 23 May 2022 11:11:59 -0400 Received: from mail-ed1-f54.google.com (mail-ed1-f54.google.com [209.85.208.54]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4DDAC47AEC; Mon, 23 May 2022 08:11:57 -0700 (PDT) Received: by mail-ed1-f54.google.com with SMTP id c10so19567049edr.2; Mon, 23 May 2022 08:11:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=0QPzY5dv4g1MBNtDQB96ePR1IDzyzraTlb4TcmcBYLI=; b=drb7p5u1yRpP455qf5piYTpoXrza+dHWO3cX6HwTu/Cp9ZtutdaTNwvhMHrxngkXA2 z2a/m6JoN/+dNwk7NTKEiJ4RFyAiZhaTC6XPLqucHQmrxyA95AvumfbRqoVBvOrrhipH 67QIdfCuixdD/0khTRXF2Gt3iAuYvT9XTOFpnNOjF9Ab46zRfeuT4OGktMkIgyO52HYE qL6IVp7GFU36/AxJeM/UK2V7TuZA720sG/KEzF0HmI2G0tcZ+RdxOz2lbX3qD72dZejE AJLu6c1qnXvgIuVQt93KGbuUxE3oHKO+NQFpPYc7j8RJVZPQolvuxZXwhrrf1XY8D8Ot mwrQ== X-Gm-Message-State: AOAM531303ivmkc6X5ETqX2Qgtp8OEwR0qT2sNYfjOxjMDUe1o6l8/VS lDtNphR1y80nm6jp35OyDp7vE7OUxK66fbon X-Google-Smtp-Source: ABdhPJyzJtYNQF9FW8ZDSNAhOVugSa25KhyQNGZ8/xGycXW0STeEPhPlSRI9XDapoAu/NQaJ1Y1c4A== X-Received: by 2002:a50:fe8d:0:b0:42a:b3a4:6ac2 with SMTP id d13-20020a50fe8d000000b0042ab3a46ac2mr24108454edt.131.1653318714696; Mon, 23 May 2022 08:11:54 -0700 (PDT) Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com. [209.85.221.45]) by smtp.gmail.com with ESMTPSA id ci18-20020a170907267200b006fe8c831632sm5794165ejc.73.2022.05.23.08.11.54 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 23 May 2022 08:11:54 -0700 (PDT) Received: by mail-wr1-f45.google.com with SMTP id f2so21873480wrc.0; Mon, 23 May 2022 08:11:54 -0700 (PDT) X-Received: by 2002:a05:6512:1520:b0:443:ec43:5fe8 with SMTP id bq32-20020a056512152000b00443ec435fe8mr16745443lfb.589.1653318703688; Mon, 23 May 2022 08:11:43 -0700 (PDT) MIME-Version: 1.0 References: <20220522155046.260146-1-tmaimon77@gmail.com> <20220522155046.260146-12-tmaimon77@gmail.com> <86cd6a37-70ad-3a90-bc8a-dcd8b41f1175@linaro.org> <62562cdf-93e3-f642-5bbd-48329eff33ea@linaro.org> In-Reply-To: <62562cdf-93e3-f642-5bbd-48329eff33ea@linaro.org> From: Geert Uytterhoeven Date: Mon, 23 May 2022 17:11:30 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v1 11/19] dt-bindings: reset: npcm: Add support for NPCM8XX To: Krzysztof Kozlowski Cc: Tomer Maimon , Avi Fishman , Tali Perry , Joel Stanley , Patrick Venture , Nancy Yuen , Benjamin Fair , Rob Herring , Krzysztof Kozlowski , Michael Turquette , Stephen Boyd , Philipp Zabel , Greg KH , Daniel Lezcano , Thomas Gleixner , Wim Van Sebroeck , Guenter Roeck , Catalin Marinas , Will Deacon , Arnd Bergmann , Olof Johansson , Jiri Slaby , Shawn Guo , =?UTF-8?Q?Bj=C3=B6rn_Andersson?= , Marcel Ziswiler , Vinod Koul , Biju Das , Nobuhiro Iwamatsu , Robert Hancock , =?UTF-8?Q?Jonathan_Neusch=C3=A4fer?= , Lubomir Rintel , arm-soc , devicetree , Linux Kernel Mailing List , linux-clk , "open list:SERIAL DRIVERS" , Linux Watchdog Mailing List , Linux ARM Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Krzysztof, On Mon, May 23, 2022 at 4:26 PM Krzysztof Kozlowski wrote: > On 23/05/2022 16:22, Geert Uytterhoeven wrote: > > On Mon, May 23, 2022 at 4:03 PM Tomer Maimon wrote: > >> On Mon, 23 May 2022 at 12:01, Krzysztof Kozlowski wrote: > >>> On 22/05/2022 17:50, Tomer Maimon wrote: > >>>> Add binding document and device tree binding > >>>> constants for Nuvoton BMC NPCM8XX reset controller. > >>>> > >>>> Signed-off-by: Tomer Maimon > > > >>>> --- /dev/null > >>>> +++ b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h > >>>> @@ -0,0 +1,124 @@ > >>>> +/* SPDX-License-Identifier: GPL-2.0 */ > >>>> +// Copyright (c) 2022 Nuvoton Technology corporation. > >>>> + > >>>> +#ifndef _DT_BINDINGS_NPCM8XX_RESET_H > >>>> +#define _DT_BINDINGS_NPCM8XX_RESET_H > >>>> + > >>>> +#define NPCM8XX_RESET_IPSRST1 0x20 > >>>> +#define NPCM8XX_RESET_IPSRST2 0x24 > >>>> +#define NPCM8XX_RESET_IPSRST3 0x34 > >>>> +#define NPCM8XX_RESET_IPSRST4 0x74 > >>> > >>> What are these? All IDs should be incremental, decimal and start from 0. > >> > >> Register offset, we use the same method in NPCM7xx. please refer > >> https://elixir.bootlin.com/linux/v5.18/source/include/dt-bindings/reset/nuvoton,npcm7xx-reset.h > >> > >> and the driver asserts the reset according to the reset include definitions > > > > So if they're easy to look up the values, you could do without the > > definitions? Cfr. the interrupts properties in .dtsi files, where we > > typically just use the hardcoded numbers. > > > > If you do decide to keep them, a comment explaining their origins > > would be useful. > > > >>>> + > >>>> +/* Reset lines on IP1 reset module (NPCM8XX_RESET_IPSRST1) */ > >>>> +#define NPCM8XX_RESET_GDMA0 3 > >>> > >>> IDs start from 0 and do not have holes. > >> > >> This represents the reset BIT in the reset register. > > > > Likewise, I think it's a good idea to document that in a comment, cfr. > > https://elixir.bootlin.com/linux/v5.18/source/include/dt-bindings/power/r8a7795-sysc.h#L8 > > Renesas is also doing it not correct (just like many others). The > bindings are not for register bits or offsets. Such data can be DTS but > not part of bindings. I think you are taking a too-extremist standpoint. The two extremes are: 1. Numbers correspond to hardware numbers, and are easy to look up in the hardware documentation (e.g. GIC SPI interrupt numbers). => Use the hardcoded numbers in DTS. 2. Numbers do not correspond to hardware numbers, so we had to invent our own definitions and numbers, usually loosely based on some table in the hardware documentation. The driver will have to look up the numbers in a data structure, to know how to program the hardware. The numbers become part of the DT ABI, and cannot be changed (header file is append-only). => Use the binding definitions in DTS. We are taking the middle ground: there is a one-to-one relation between numbers and hardware numbers that can be looked up in or derived from the hardware documentation, but the conversion is non-trivial (for the casual human reviewer), or the documentation refers to names instead of numbers in most sections (e.g. named power domains). Then why not let the numbers match some feature in the hardware (e.g. register offset or register bit)? > Imagine now you made mistake in this register > offset and hardware uses slightly different value. What now? Change > bindings? No. Bindings hold here ID, the abstraction, and ID stays fixed. I see no difference here with using the wrong interrupt number in an interrupts property in DTS. What do we do in that case? Fix the DTS. BTW, are you aware of any driver that transforms interrupt numbers obtained from DTS, because the DTS used the wrong number? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds 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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 54958C433F5 for ; Mon, 23 May 2022 15:13:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc: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=5VpEkFn4pvqqb8uWTwyaY3mCgF4tZN5WhlrSS7pJxJI=; b=WrXGzPDhHV7FeL 0uHXZ30aRpMGcrrnA7KhBtDcEEnsjvwVcewuTtn+JKmSBEkiMaumiulG+gEi3GCayuz/1jzw0eLbY 2+KjEzTwRU2EGl50KHJFOca3uJ5QgufinRhUeSy1FipIKhKeSTjEtl8PkXVRCddjME/zrjZ97cXBK BUBWOdg7oMC8uwh9dws9F1q9gVOyZ2eHwiwiXSWzPy2mL3w1sdtpv/IdwQOHXmBnnOEcQAbqprlAO sDZqzH7ICaUbD2xMV2Gx01T3lGV7bEzkMKJH5zzYu3Cd+7c+gRyIeLGEuC1G/iYjuDNkIrZvkNesB ylo2io4F9pUn5KuvU06Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nt9iz-004sH2-Dv; Mon, 23 May 2022 15:12:01 +0000 Received: from mail-ej1-f46.google.com ([209.85.218.46]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nt9iv-004sFx-Nf for linux-arm-kernel@lists.infradead.org; Mon, 23 May 2022 15:11:59 +0000 Received: by mail-ej1-f46.google.com with SMTP id f9so29563863ejc.0 for ; Mon, 23 May 2022 08:11:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=0QPzY5dv4g1MBNtDQB96ePR1IDzyzraTlb4TcmcBYLI=; b=hLHFILX6tfAbnkOJDI9oRgG48CORSGwSn1YNQfRGmQcWwXBFVZAkTBvAyue200upWL QHI1GMftnKqyzL2IH1Y4bUyPCjfhTbBFYk3OBHWZRpMUJETk2ej2ltFOnPfvzqeqvGQj GuzwFiBCwb4gGhqVAhVKq3I+GgyU5oQG8rVe4Nns3q8NnTRO9Mpr9kZ3h6aIE/vBQC1X jey+HHb1vL5jPsEQhBnZzyUPpT7InPmbp9DFWU+90IW/rCysap6QD7y7ggW7mSrc0n5C FK1h6E9NnyniW/WFd3XmTD2MvDJ6951/T1OFrWm79FDfNr7tqoKpWSq6nbps0MS6MuoM UYwQ== X-Gm-Message-State: AOAM531S7w7m5ADIs/leUefhIZgOlVvsZ3f5kqYUPyPl1Q02FueNZXop 0r3/WI+WZE5WetYm5ZB9fIoQitBaaqYvi5ap X-Google-Smtp-Source: ABdhPJxgcK+1zmg9ZGXpn2kCXi6I3ehRCweqAA8MxsPhAsC4ptgpiBvl26ME4fm8BzD5D2w6lE2Taw== X-Received: by 2002:a17:907:720f:b0:6f8:5c31:4027 with SMTP id dr15-20020a170907720f00b006f85c314027mr19958048ejc.284.1653318714449; Mon, 23 May 2022 08:11:54 -0700 (PDT) Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com. [209.85.221.46]) by smtp.gmail.com with ESMTPSA id qb18-20020a1709077e9200b006feed212f41sm945598ejc.67.2022.05.23.08.11.54 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 23 May 2022 08:11:54 -0700 (PDT) Received: by mail-wr1-f46.google.com with SMTP id r23so21869650wrr.2 for ; Mon, 23 May 2022 08:11:54 -0700 (PDT) X-Received: by 2002:a05:6512:1520:b0:443:ec43:5fe8 with SMTP id bq32-20020a056512152000b00443ec435fe8mr16745443lfb.589.1653318703688; Mon, 23 May 2022 08:11:43 -0700 (PDT) MIME-Version: 1.0 References: <20220522155046.260146-1-tmaimon77@gmail.com> <20220522155046.260146-12-tmaimon77@gmail.com> <86cd6a37-70ad-3a90-bc8a-dcd8b41f1175@linaro.org> <62562cdf-93e3-f642-5bbd-48329eff33ea@linaro.org> In-Reply-To: <62562cdf-93e3-f642-5bbd-48329eff33ea@linaro.org> From: Geert Uytterhoeven Date: Mon, 23 May 2022 17:11:30 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v1 11/19] dt-bindings: reset: npcm: Add support for NPCM8XX To: Krzysztof Kozlowski Cc: Tomer Maimon , Avi Fishman , Tali Perry , Joel Stanley , Patrick Venture , Nancy Yuen , Benjamin Fair , Rob Herring , Krzysztof Kozlowski , Michael Turquette , Stephen Boyd , Philipp Zabel , Greg KH , Daniel Lezcano , Thomas Gleixner , Wim Van Sebroeck , Guenter Roeck , Catalin Marinas , Will Deacon , Arnd Bergmann , Olof Johansson , Jiri Slaby , Shawn Guo , =?UTF-8?Q?Bj=C3=B6rn_Andersson?= , Marcel Ziswiler , Vinod Koul , Biju Das , Nobuhiro Iwamatsu , Robert Hancock , =?UTF-8?Q?Jonathan_Neusch=C3=A4fer?= , Lubomir Rintel , arm-soc , devicetree , Linux Kernel Mailing List , linux-clk , "open list:SERIAL DRIVERS" , Linux Watchdog Mailing List , Linux ARM X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220523_081157_816296_8AAB8EC3 X-CRM114-Status: GOOD ( 42.63 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Krzysztof, On Mon, May 23, 2022 at 4:26 PM Krzysztof Kozlowski wrote: > On 23/05/2022 16:22, Geert Uytterhoeven wrote: > > On Mon, May 23, 2022 at 4:03 PM Tomer Maimon wrote: > >> On Mon, 23 May 2022 at 12:01, Krzysztof Kozlowski wrote: > >>> On 22/05/2022 17:50, Tomer Maimon wrote: > >>>> Add binding document and device tree binding > >>>> constants for Nuvoton BMC NPCM8XX reset controller. > >>>> > >>>> Signed-off-by: Tomer Maimon > > > >>>> --- /dev/null > >>>> +++ b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h > >>>> @@ -0,0 +1,124 @@ > >>>> +/* SPDX-License-Identifier: GPL-2.0 */ > >>>> +// Copyright (c) 2022 Nuvoton Technology corporation. > >>>> + > >>>> +#ifndef _DT_BINDINGS_NPCM8XX_RESET_H > >>>> +#define _DT_BINDINGS_NPCM8XX_RESET_H > >>>> + > >>>> +#define NPCM8XX_RESET_IPSRST1 0x20 > >>>> +#define NPCM8XX_RESET_IPSRST2 0x24 > >>>> +#define NPCM8XX_RESET_IPSRST3 0x34 > >>>> +#define NPCM8XX_RESET_IPSRST4 0x74 > >>> > >>> What are these? All IDs should be incremental, decimal and start from 0. > >> > >> Register offset, we use the same method in NPCM7xx. please refer > >> https://elixir.bootlin.com/linux/v5.18/source/include/dt-bindings/reset/nuvoton,npcm7xx-reset.h > >> > >> and the driver asserts the reset according to the reset include definitions > > > > So if they're easy to look up the values, you could do without the > > definitions? Cfr. the interrupts properties in .dtsi files, where we > > typically just use the hardcoded numbers. > > > > If you do decide to keep them, a comment explaining their origins > > would be useful. > > > >>>> + > >>>> +/* Reset lines on IP1 reset module (NPCM8XX_RESET_IPSRST1) */ > >>>> +#define NPCM8XX_RESET_GDMA0 3 > >>> > >>> IDs start from 0 and do not have holes. > >> > >> This represents the reset BIT in the reset register. > > > > Likewise, I think it's a good idea to document that in a comment, cfr. > > https://elixir.bootlin.com/linux/v5.18/source/include/dt-bindings/power/r8a7795-sysc.h#L8 > > Renesas is also doing it not correct (just like many others). The > bindings are not for register bits or offsets. Such data can be DTS but > not part of bindings. I think you are taking a too-extremist standpoint. The two extremes are: 1. Numbers correspond to hardware numbers, and are easy to look up in the hardware documentation (e.g. GIC SPI interrupt numbers). => Use the hardcoded numbers in DTS. 2. Numbers do not correspond to hardware numbers, so we had to invent our own definitions and numbers, usually loosely based on some table in the hardware documentation. The driver will have to look up the numbers in a data structure, to know how to program the hardware. The numbers become part of the DT ABI, and cannot be changed (header file is append-only). => Use the binding definitions in DTS. We are taking the middle ground: there is a one-to-one relation between numbers and hardware numbers that can be looked up in or derived from the hardware documentation, but the conversion is non-trivial (for the casual human reviewer), or the documentation refers to names instead of numbers in most sections (e.g. named power domains). Then why not let the numbers match some feature in the hardware (e.g. register offset or register bit)? > Imagine now you made mistake in this register > offset and hardware uses slightly different value. What now? Change > bindings? No. Bindings hold here ID, the abstraction, and ID stays fixed. I see no difference here with using the wrong interrupt number in an interrupts property in DTS. What do we do in that case? Fix the DTS. BTW, are you aware of any driver that transforms interrupt numbers obtained from DTS, because the DTS used the wrong number? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel