On Wed, Nov 10, 2021 at 12:01:19AM +0000, Daniel Scally wrote:
> On 09/11/2021 16:35, Daniel Scally wrote:
Some comments to the code below.
...
> +static int dw9719_i2c_rd8(struct i2c_client *client, u8 reg, u8 *val)
> +{
> + struct i2c_msg msg[2];
> + u8 buf[2] = { reg };
See below.
> + int ret;
> +
> + msg[0].addr = client->addr;
> + msg[0].flags = 0;
> + msg[0].len = 1;
> + msg[0].buf = buf;
> +
> + msg[1].addr = client->addr;
> + msg[1].flags = I2C_M_RD;
> + msg[1].len = 1;
> + msg[1].buf = &buf[1];
> + *val = 0;
> +
> + ret = i2c_transfer(client->adapter, msg, 2);
> + if (ret < 0)
> + goto err;
> +
> + *val = buf[1];
> +
> + return 0;
> +err:
> + return ret;
Useless. Return in-place.
> +}
...
> + u8 buf[3] = { reg, (u8)(val >> 8), (u8)(val & 0xff)};
This, and similar cases, has endianess issue.
You are supposed to have __be16 or __le16 buffer with respect to the hardware.
Another way (since I looked at the other places) is to use put_unligned_*().
As per above this requires put_unaligned_be16().
...
> + pm_runtime_set_autosuspend_delay(&client->dev, 1000);
Why this can't be set by user space?
...
> Subject: [PATCH 2/3] device property: Check fwnode->secondary when finding
> properties
>
> fwnode_property_get_reference_args() searches for named properties
> against a fwnode_handle, but these could instead be against the fwnode's
> secondary. If the property isn't found against the primary, check the
> secondary to see if it's there instead.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Signed-off-by: Daniel Scally <djrscally@xxxxxxxxx>
> ---
> drivers/base/property.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 453918eb7390..054e62a4e710 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -479,8 +479,16 @@ int fwnode_property_get_reference_args(const struct fwnode_handle *fwnode,
> unsigned int nargs, unsigned int index,
> struct fwnode_reference_args *args)
> {
> - return fwnode_call_int_op(fwnode, get_reference_args, prop, nargs_prop,
> - nargs, index, args);
> + int ret;
> +
> + ret = fwnode_call_int_op(fwnode, get_reference_args, prop, nargs_prop,
> + nargs, index, args);
> +
> + if (ret < 0 && !IS_ERR_OR_NULL(fwnode->secondary))
> + ret = fwnode_call_int_op(fwnode->secondary, get_reference_args,
> + prop, nargs_prop, nargs, index, args);
> +
> + return ret;
> }
> EXPORT_SYMBOL_GPL(fwnode_property_get_reference_args);
>
> --
> 2.25.1
>
> From dd7532ddea71482502394b6b36c9fd3e5f2a0a37 Mon Sep 17 00:00:00 2001
> From: Daniel Scally <djrscally@xxxxxxxxx>
> Date: Tue, 9 Nov 2021 23:12:06 +0000
> Subject: [PATCH 1/3] platform/x86: int3472: Add vsio regulator supply to board
> file
>
> The Surface Go2 board file needs to additionally specify a supply name
> mapping the VSIO regulator to the world facing camera's VCM device, as
> it can sit behind an I2C daisy chain which requires this regulator be
> enabled to function.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Signed-off-by: Daniel Scally <djrscally@xxxxxxxxx>
> ---
> drivers/platform/x86/intel/int3472/tps68470_board_data.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/platform/x86/intel/int3472/tps68470_board_data.c b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> index 20615c342875..556a615afaa9 100644
> --- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> +++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> @@ -29,6 +29,7 @@ static struct regulator_consumer_supply int347a_vcm_consumer_supplies[] = {
>
> static struct regulator_consumer_supply int347a_vsio_consumer_supplies[] = {
> REGULATOR_SUPPLY("dovdd", "i2c-INT347A:00"),
> + REGULATOR_SUPPLY("vsio", "i2c-INT347A:00-VCM"),
> };
>
> static const struct regulator_init_data surface_go_tps68470_core_reg_init_data = {
> --
> 2.25.1
>
--
With Best Regards,
Andy Shevchenko
