On Tue, Nov 18, 2025 at 08:37:22AM +0100, Krzysztof Kozlowski wrote:
> On 18/11/2025 05:37, Vishnu Saini wrote:
> > +static void lt8713sx_reset(struct lt8713sx *lt8713sx)
> > +{
> > + pr_debug("reset bridge.\n");
> > + gpiod_set_value_cansleep(lt8713sx->reset_gpio, 1);
> > + msleep(20);
> > +
> > + gpiod_set_value_cansleep(lt8713sx->reset_gpio, 0);
> > + msleep(20);
> > +
> > + gpiod_set_value_cansleep(lt8713sx->reset_gpio, 1);
> > + msleep(20);
> > + pr_debug("reset done.\n");
>
> No, it is not done, because you kept the device in the reset. 1 is
> reset. Don't mix up line and logical signals.
done.
> > +}
> > +
> > +static int lt8713sx_regulator_init(struct lt8713sx *lt8713sx)
> > +{
> > + int ret;
> > +
> > + lt8713sx->supplies[0].supply = "vdd";
> > + lt8713sx->supplies[1].supply = "vcc";
> > +
> > + ret = devm_regulator_bulk_get(lt8713sx->dev, 2, lt8713sx->supplies);
> > + if (ret < 0)
> > + return dev_err_probe(lt8713sx->dev, ret, "failed to get
> > regulators\n");
> > +
> > + ret = regulator_set_load(lt8713sx->supplies[0].consumer, 200000);
> > + if (ret < 0)
> > + return dev_err_probe(lt8713sx->dev, ret, "failed to set
> > regulator load\n");
> > +
> > + return 0;
> > +}
> > +
> > +static int lt8713sx_regulator_enable(struct lt8713sx *lt8713sx)
> > +{
> > + int ret;
> > +
> > + ret = regulator_enable(lt8713sx->supplies[0].consumer);
> > + if (ret < 0)
> > + return dev_err_probe(lt8713sx->dev, ret, "failed to enable vdd
> > regulator\n");
> > +
> > + usleep_range(1000, 10000);
> > +
> > + ret = regulator_enable(lt8713sx->supplies[1].consumer);
> > + if (ret < 0) {
> > + regulator_disable(lt8713sx->supplies[0].consumer);
> > + return dev_err_probe(lt8713sx->dev, ret, "failed to enable vcc
> > regulator\n");
> > + }
> > + return 0;
> > +}
> > +
> > +static int lt8713sx_gpio_init(struct lt8713sx *lt8713sx)
> > +{
> > + struct device *dev = lt8713sx->dev;
> > +
> > + lt8713sx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> > + if (IS_ERR(lt8713sx->reset_gpio))
> > + return dev_err_probe(dev, PTR_ERR(lt8713sx->reset_gpio),
> > + "failed to acquire reset gpio\n");
> > +
> > + /* power enable gpio */
> > + lt8713sx->enable_gpio = devm_gpiod_get_optional(dev, "enable",
> > GPIOD_OUT_HIGH);
> > + if (IS_ERR(lt8713sx->enable_gpio))
> > + return dev_err_probe(dev, PTR_ERR(lt8713sx->enable_gpio),
> > + "failed to acquire enable gpio\n");
> > + return 0;
> > +}
> > +
> > +static ssize_t lt8713sx_firmware_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t len)
> > +{
> > + struct lt8713sx *lt8713sx = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + ret = lt8713sx_firmware_update(lt8713sx);
> > + if (ret < 0)
> > + return ret;
> > + return len;
> > +}
> > +
> > +static DEVICE_ATTR_WO(lt8713sx_firmware);
> > +
> > +static struct attribute *lt8713sx_attrs[] = {
> > + &dev_attr_lt8713sx_firmware.attr,
> > + NULL,
> > +};
> > +
> > +static const struct attribute_group lt8713sx_attr_group = {
> > + .attrs = lt8713sx_attrs,
> > +};
> > +
> > +static const struct attribute_group *lt8713sx_attr_groups[] = {
> > + <8713sx_attr_group,
> > + NULL,
> > +};
> > +
> > +static int lt8713sx_probe(struct i2c_client *client)
> > +{
> > + struct lt8713sx *lt8713sx;
> > + struct device *dev = &client->dev;
> > + int ret;
> > +
> > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> > + return dev_err_probe(dev, -ENODEV, "device doesn't support
> > I2C\n");
> > +
> > + lt8713sx = devm_kzalloc(dev, sizeof(*lt8713sx), GFP_KERNEL);
> > + if (!lt8713sx)
> > + return dev_err_probe(dev, -ENOMEM, "failed to allocate lt8713sx
> > struct\n");
> > +
> I did not ask for dev_err_probe here. Do you see such pattern anywhere?
> No, because there are never error messages on memory allocation (see
> coccinelle). Drop.
Dropped this dev_err_probe.
> Please run standard kernel tools for static analysis, like coccinelle,
> smatch and sparse, and fix reported warnings. Also please check for
> warnings when building with W=1 for gcc and clang. Most of these
> commands (checks or W=1 build) can build specific targets, like some
> directory, to narrow the scope to only your code. The code here looks
> like it needs a fix. Feel free to get in touch if the warning is not clear.
Run a couple of tools you mentioned and fixed the warnings in next patch.
> Best regards,
> Krzysztof
