>> +static int cgbc_gpio_get(struct gpio_chip *chip, unsigned int offset)
>> +{
>> + struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
>> + struct cgbc_device_data *cgbc = gpio->cgbc;
>> + int ret;
>> + u8 val;
>> +
>
> Can you use scoped_guard() here and elsewhere?
Hi Bartosz,
Thanks for the review.
For the next iteration I added scoped_guard() in cgbc_gpio_get(), and
guard() in cgbc_gpio_set(), cgbc_gpio_direction_input(), and
cgbc_gpio_direction_output().
>
>> + mutex_lock(&gpio->lock);
>> +
>> + ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_GET, (offset > 7) ? 1 : 0, 0, &val);
>> +
>> + mutex_unlock(&gpio->lock);
>> +
>> + offset %= 8;
>> +
>> + if (ret)
>> + return ret;
...
>> +static int cgbc_gpio_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct cgbc_device_data *cgbc = dev_get_drvdata(dev->parent);
>> + struct cgbc_gpio_data *gpio;
>> + struct gpio_chip *chip;
>> + int ret;
>> +
>> + gpio = devm_kzalloc(dev, sizeof(*gpio), GFP_KERNEL);
>> + if (!gpio)
>> + return -ENOMEM;
>> +
>> + gpio->cgbc = cgbc;
>> +
>> + platform_set_drvdata(pdev, gpio);
>> +
>> + chip = &gpio->chip;
>> + chip->label = dev_name(&pdev->dev);
>> + chip->owner = THIS_MODULE;
>> + chip->parent = dev;
>> + chip->base = -1;
>> + chip->direction_input = cgbc_gpio_direction_input;
>> + chip->direction_output = cgbc_gpio_direction_output;
>> + chip->get_direction = cgbc_gpio_get_direction;
>> + chip->get = cgbc_gpio_get;
>> + chip->set = cgbc_gpio_set;
>> + chip->ngpio = CGBC_GPIO_NGPIO;
>> +
>> + mutex_init(&gpio->lock);
>
> Please use devm_mutex_init() so that it gets cleaned up at exit. It's
> not strictly necessary but helps with lock debugging.
Fixed in the next iteration.
Regards,
Thomas
--
Thomas Richard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
