Re: [PATCH V3 2/2] Add support for Awinic proximity sensor

Jonathan Cameron
2024-07-27T12:22:35Z

> >> diff --git a/drivers/iio/proximity/aw9610x.c b/drivers/iio/proximity/aw9610x.c
> >> new file mode 100644
> >> index 000000000000..15e53d55d2a1
> >> --- /dev/null
> >> +++ b/drivers/iio/proximity/aw9610x.c
> >> @@ -0,0 +1,1150 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * AWINIC sar sensor driver (aw9610x)
> >> + *
> >> + * Author: Shuaijie Wang<wangshuaijie@awinic.com>
> >> + *
> >> + * Copyright (c) 2024 awinic Technology CO., LTD
> >> + */
> >> +#include "aw_sar.h"
> >> +
> >> +#define AW9610X_I2C_NAME "aw9610x_sar"
> >> +#define AW9610X_CHANNEL_MAX (5)
> >
> >No brackets around numeric values. They add nothing.
> >
>
> The patch for v4 will fix these issues.

If you agree with a comment, usual convention to save everyone time is
don't reply to it and ideally crop that part of the email out entirely.

It is hard to find the active discussions that need replies in an
email unless it is cropped to make them the only remaining parts.

...

>
> >> +
> >> +#define AFE_BASE_ADDR (0x0000)
> >
> The patch for v4 will fix these issues.
>
> >> +
> >> + if (reg_val != AW9610X_CHIP_ID) {
> >> + dev_err(p_sar->dev, "unsupport dev, chipid is (0x%04x)", reg_val);
> >
> >
> >To allow use of fallback compatibles in DT we normally accept chipid missmatches.
> >So at most dev_info and carry on anyway.
> >
>
> Sorry, if the chipid does not match, the driver is likely to encounter issues
> during operation. We will not consider compatibility with more devices for
> the time being.

This discussion has been had many times and the standard approach to this has
changed over time (in IIO anyway). We have not yet updated all drivers to
the current policy.

Current policy is that we do not block probing a chip that firmware has told
us is compatible on the basis of a failure to match a chip id.

Print a message to help with any bug report issues you may get.
+ educate integrators of your device that they must not put false compatibles
in their firmware.

>
> >> + return -EINVAL;
> >> + }
> >> + memcpy(p_sar->chip_name, "AW9610X", 8);
> >> +
> >> + return 0;
> >> +}
...

> Kind regards,
> Wang Shuaijie