On Mon, Jun 10, 2024 at 10:46:26AM +0200, Krzysztof Kozlowski wrote:
> > +
> > + cs530x = devm_kzalloc(&client->dev, sizeof(struct cs530x_priv),
>
> sizeof(*)
>
Ack
> > +
> > + if (client->dev.of_node) {
> > + const struct of_device_id *match;
> > +
> > + match = of_match_node(cs530x_of_match, client->dev.of_node);
> > + if (match == NULL)
> > + return -EINVAL;
> > + cs530x->devtype = (enum cs530x_type)match->data;
>
> I think you open-coded device_get_match_data
>
>
> > + } else {
> > + const struct i2c_device_id *id =
> > + i2c_match_id(cs530x_i2c_id, client);
> > +
> > + cs530x->devtype = id->driver_data;
> > + }
>
> and for all of this there is a helper i2c_get_device_match_data.
>
Ack
> > + if (ret != 0) {
> > + dev_err(dev, "Failed to request supplies: %d\n", ret);
>
> return dev_err_probe()
Ack
> > + if (ret != 0) {
> > + dev_err(dev, "Failed to enable supplies: %d\n", ret);
>
> return dev_err_probe()
Ack
> > + if (IS_ERR(cs530x->reset_gpio)) {
> > + dev_err(dev, "Reset gpio not available\n");
>
> return dev_err_probe()
Ack
> > +
> > + if (cs530x->reset_gpio) {
> > + usleep_range(2000, 2100);
> > + gpiod_set_value_cansleep(cs530x->reset_gpio, 1);
>
> 1 is to keep device in reset? This looks like you are using logical
> values inverted.
Thanks. The reset pin is active low. I will also change the dt binding
example to reflect that.
> > +
> > + cs530x->dev_dai = devm_kmemdup(dev, &cs530x_dai,
> > + sizeof(struct snd_soc_dai_driver),
>
> sizeof(*)
>
Ack
Regards,
Paul
