Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 29 Jul 2022 09:16:06 +0200
From:   Henning Schild <henning.schild@siemens.com>
To:     Andy Shevchenko <andy.shevchenko@gmail.com>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        Bartosz Golaszewski <brgl@bgdev.pl>,
        Linus Walleij <linus.walleij@linaro.org>,
        Tasanakorn Phaipool <tasanakorn@gmail.com>,
        Sheng-Yuan Huang <syhuang3@nuvoton.com>,
        Pavel Machek <pavel@ucw.cz>
Subject: Re: [PATCH 1/4] gpio-f7188x: Add GPIO support for Nuvoton NCT6116
Message-ID: <20220729091606.1205dcdf@md1za8fc.ad001.siemens.net>
In-Reply-To: <CAHp75VcOHxJDBpnLrMQOWRED-WfJebRrYPtJZ35-8_B6wHAi1w@mail.gmail.com>
References: <20220728155652.29516-1-henning.schild@siemens.com>
        <20220728155652.29516-2-henning.schild@siemens.com>
        <CAHp75VcOHxJDBpnLrMQOWRED-WfJebRrYPtJZ35-8_B6wHAi1w@mail.gmail.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?BAnxIi7H1uAN7J4pqxkENx3LfDHtqGwqSf6cGkOAyR29EwZhXmib4wdQgxSE?=
 =?us-ascii?Q?6JxaUp2xdgYLQkMgBMEb/mCP0NwKiiXIRVVrit3d6esTY+NhIYtQhl9eACoE?=
 =?us-ascii?Q?4L79E2igo7056BG3gnNS2s9sxTleZRVL8qWGWuDQ7R9wueqYAILj/WXT0fMe?=
 =?us-ascii?Q?OuyNqTP/Y020cSEQ2/oOQl+Y/bI+otnWTmGnvjHFkT1KeU2JBS3DdJ0WvqJu?=
 =?us-ascii?Q?fLcOG/NJIwxe21B3+5l3e8bo187U7lpq0r5ZY4EOzSp3vcMSm+04pw/ndnq/?=
 =?us-ascii?Q?deh+MAxqRQ/nBtPpoT5i6SIMM/LHjvHkOC4w4rSavxUQvpqjev1SBBUE3VLF?=
 =?us-ascii?Q?NwTNHDVlLnswwo5VDsfIpkwgLy3GoG32LsxMg8cD5YUoQHtH/ACTC3Pt8+3g?=
 =?us-ascii?Q?eWxxPVVZM8bR+rU7sr/Lz+AbI68Hjzjw1Yc8PJPoup/cqFgWKdhgw1eAGatd?=
 =?us-ascii?Q?i7urGv3+T8dv+/9BNJxwsbMXri5ksquwH1x/XuwtSRJ2RmfG1d+O17RL8yrq?=
 =?us-ascii?Q?nbVbIkG5Gry8Yx23FhRxwjzQexQwk89NXaTO1xT2EmTenqlHD7OTnShlggx7?=
 =?us-ascii?Q?6PuRdqWSQPfXq3maaL8H3MfjQj+cMzkxzEc+h9m0Q2uekjHnecN75N2OSglg?=
 =?us-ascii?Q?LhLB3O8oQOWzsURzyvYydbhvDR1dvL1Oh3JXNF0Ye7n1ytnpAo6dgyFJlvLe?=
 =?us-ascii?Q?GAtg55MT9J3klXXJ2Xcde/jbjiC/R7qHOPqwk1A6cTiUJACkUtfOzmtjcfNk?=
 =?us-ascii?Q?AnQOaDQrRnW5mRy+qWSNJjFCBoT3eUWGnbUVOSpdwwtdceYQgqRdUVdZy65d?=
 =?us-ascii?Q?GrCyB1Jrhx/wIKu0PuIfAAWEvQt0CohrUu2DrCiYUPJTaiVrdGcLXN3Ho3P+?=
 =?us-ascii?Q?VkcKKkWrdwzxMXJ/LggPEMam2N7FVklZFz086NIJ4qti8+NSK3vzYeU8klAv?=
 =?us-ascii?Q?70toVuo8KTJIozf2IT6uA3Nf3JibJSF0kYCmm0CshIDCKGWdBJ16/9L2f95D?=
 =?us-ascii?Q?plUEsjSni7HHs5h5WDY6c0oRXvlgfE2vd3qIOJN1+Qb0llj4EeetQQMlNvyM?=
 =?us-ascii?Q?PjSpNpgdyHeMK0NhnuXsGBZxKcjax+v/r6dEFe1ugbCotcwNXyOG+ZKgFlNV?=
 =?us-ascii?Q?+Gt3jOJucHj5ys+N1axC9V8PZTNvK2o/k40C+2+51Z7piJ693EZBkg+NwTs5?=
 =?us-ascii?Q?q6aE/WUD8RN6kF10kIC2UhmxP7Qdw+SnluTxlAlz9qJr6Ggd7rfqzYbYSdw1?=
 =?us-ascii?Q?5pl+MT5zJrzi9mXtWwZNSAf5L9PdCQ9Q7x7FEiRdTjcuRNj0DlVibKXU5KyU?=
 =?us-ascii?Q?P7KzN/qQlQcwbcHPi1Fnn9jHPYXYICHybf12yjRWBy70lkig+sJH2GfTjOzV?=
 =?us-ascii?Q?vstzco3I5MKGpbNIsgxsxQoEF8yFhic0vA3E7zdr4x/FNLKJ1UYT2AGICqrB?=
 =?us-ascii?Q?FPftSlBHV70E9aUATBT/DgxMkDa38kfQ6zWVTUKH8M5Ern6oIgxGUeA6yMbF?=
 =?us-ascii?Q?D6+5jvQXcuql/WJYvPZZ4pn6XL4AhALjDHTTp27Sj449KpKUNGlUvK8Atei1?=
 =?us-ascii?Q?fA2doCfp65uJysmL/O2ybA7QGEdvC54K7XZ+YeHcwLrRZ1fsKdeBiyy1FMaW?=
 =?us-ascii?Q?VV2u1vtOSzgkqen8IuB1gEBOQTilRG4UGs4N462AoOi1O6d3jIorDxmgDE+8?=
 =?us-ascii?Q?0dr2Fw=3D=3D?=
X-OriginatorOrg: siemens.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 91c3e5ad-980e-4ad0-b431-08da713236b9
X-MS-Exchange-CrossTenant-AuthSource: PA4PR10MB5780.EURPRD10.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jul 2022 07:16:10.8195
 (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: yYmoBD0Cl8vTS2HuE+qITNN2MCw9Vr8i7O8/2eMAokPm4TgR/x2stN95WziB2wLv8ZIYU7rH0XHxELFoh+d5NtxLvmEFgxqufuJxl/5eods=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR10MB5572
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_MED,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE
        autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am Thu, 28 Jul 2022 23:33:36 +0200
schrieb Andy Shevchenko <andy.shevchenko@gmail.com>:

> On Thu, Jul 28, 2022 at 5:57 PM Henning Schild
> <henning.schild@siemens.com> wrote:
> >
> > Add GPIO support for Nuvoton NCT6116 chip. Nuvoton SuperIO chips are
> > very similar to the ones from Fintek. In other subsystems they also
> > share drivers and are called a family of drivers.
> >
> > For the GPIO subsystem the only difference is that the direction
> > bit is reversed and that there is only one data bit per pin. On the
> > SuperIO level the logical device is another one.  
> 
> ...
> 
> > +#define SIO_GPIO_ENABLE                0x30    /* GPIO enable */  
> 
> I don't see how it's being utilized... (But okay, it might be good to
> have as a hint for a reader who has no access to the documentation).

Good catch. That is a leftover from code that turned out to be not
needed. Will drop.

> ...
> 
> > +       if (sio->device == SIO_LD_GPIO_NUVOTON) {  
> 
> Everywhere else you use `device == SIO_LD_GPIO_FINTEK`, perhaps here
> for consistency? However, I would rather see a field that clearly
> states that it's an inverted value. Then you can use
> 
>   if (sio->dir_inv)
>     ...do something...

Good idea, will look into that. Given we talk about a family of chips
there might be more vendor ids that should be mapped onto "inv" in the
future.

Henning

> > +               if (dir & BIT(offset))
> > +                       return GPIO_LINE_DIRECTION_IN;
> > +
> > +               return GPIO_LINE_DIRECTION_OUT;
> > +       }
> > +
> > +       if (dir & BIT(offset))
> >                 return GPIO_LINE_DIRECTION_OUT;
> >
> >         return GPIO_LINE_DIRECTION_IN;  
>