Received-SPF: pass (google.com: domain of linux-kernel+bounces-145313-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240415-pinctrl-scmi-v10-0-59c6e7a586ee@nxp.com> <20240415-pinctrl-scmi-v10-4-59c6e7a586ee@nxp.com>
In-Reply-To: <20240415-pinctrl-scmi-v10-4-59c6e7a586ee@nxp.com>
From: Andy Shevchenko <andy.shevchenko@gmail.com>
Date: Mon, 15 Apr 2024 17:00:31 +0300
Message-ID: <CAHp75VdoaL-66vDFeDWXg5V0XnL45F_JQZ_BNeaaOcSwQz5gnQ@mail.gmail.com>
Subject: Re: [PATCH v10 4/4] pinctrl: Implementation of the generic
 scmi-pinctrl driver
To: "Peng Fan (OSS)" <peng.fan@oss.nxp.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>, Cristian Marussi <cristian.marussi@arm.com>, 
	Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, 
	Conor Dooley <conor+dt@kernel.org>, Linus Walleij <linus.walleij@linaro.org>, 
	Dan Carpenter <dan.carpenter@linaro.org>, Dhruva Gole <d-gole@ti.com>, 
	linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, 
	devicetree@vger.kernel.org, linux-gpio@vger.kernel.org, 
	Peng Fan <peng.fan@nxp.com>, Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Apr 15, 2024 at 11:43=E2=80=AFAM Peng Fan (OSS) <peng.fan@oss.nxp.c=
om> wrote:
>
> From: Peng Fan <peng.fan@nxp.com>
>
> scmi-pinctrl driver implements pinctrl driver interface and using
> SCMI protocol to redirect messages from pinctrl subsystem SDK to
> SCMI platform firmware, which does the changes in HW.

Below are some cosmetics, but in general LGTM, thanks!
Reviewed-by: Andy Shevchenko <andy@kernel.org>

..

> +#include <linux/device.h>
> +#include <linux/dev_printk.h>

The second one is guaranteed to be included by the first one, so
dev_printk.h can be removed.

> +#include <linux/err.h>

+ errno.h as ENOTSUPP is defined there and surprisingly err.h doesn't
include that.

+ mod_devicetable.h (for the ID table type definition)

> +#include <linux/module.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>

..

> +/* Define num configs, if not large than 4 use stack, else use kcalloc *=
/

kcalloc()

..

> +       ret =3D pinctrl_ops->settings_get_one(pmx->ph, pin, PIN_TYPE, typ=
e,
> +                                           &config_value);
> +       if (ret) {
> +               /* Convert SCMI error code to PINCTRL expected error code=
 */
> +               if (ret =3D=3D -EOPNOTSUPP)
> +                       ret =3D -ENOTSUPP;
> +               return ret;
> +       }

It can be split as

       ret =3D pinctrl_ops->settings_get_one(pmx->ph, pin, PIN_TYPE, type,
                                           &config_value);
       /* Convert SCMI error code to PINCTRL expected error code */
       if (ret =3D=3D -EOPNOTSUPP)
               return -ENOTSUPP;
       if (ret)
               return ret;

..

> +       ret =3D pinctrl_ops->settings_get_one(pmx->ph, group, GROUP_TYPE,=
 type,
> +                                           &config_value);
> +       if (ret) {
> +               /* Convert SCMI error code to PINCTRL expected error code=
 */
> +               if (ret =3D=3D -EOPNOTSUPP)
> +                       ret =3D -ENOTSUPP;
> +               return ret;
> +       }

As per above.

--
With Best Regards,
Andy Shevchenko