On Mon, Apr 15, 2024 at 11:43 AM Peng Fan (OSS) <peng.fan@xxxxxxxxxxx> wrote:
>
> From: Peng Fan <peng.fan@xxxxxxx>
>
> 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@xxxxxxxxxx>
...
> +#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 = pinctrl_ops->settings_get_one(pmx->ph, pin, PIN_TYPE, type,
> + &config_value);
> + if (ret) {
> + /* Convert SCMI error code to PINCTRL expected error code */
> + if (ret == -EOPNOTSUPP)
> + ret = -ENOTSUPP;
> + return ret;
> + }
It can be split as
ret = pinctrl_ops->settings_get_one(pmx->ph, pin, PIN_TYPE, type,
&config_value);
/* Convert SCMI error code to PINCTRL expected error code */
if (ret == -EOPNOTSUPP)
return -ENOTSUPP;
if (ret)
return ret;
...
> + ret = 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 == -EOPNOTSUPP)
> + ret = -ENOTSUPP;
> + return ret;
> + }
As per above.
--
With Best Regards,
Andy Shevchenko
