Re: [PATCH V1 1/2] gpio: inverter: Add virtual controller for gpio configuration

["Enrico Weigelt, metux IT consult" <lkml@xxxxxxxxx>]

On 12.06.19 08:27, Phil Reid wrote:

Hi,

>> +#include <linux/gpio.h>
> This is a legacy header.
> You probably want to include <linux/gpio/driver.h>

Just counted 754 cases of such includes, 289 in arch, 345 in drivers.

Shall them be replaced ? Or is the new include only meant for gpio
*drivers* ?

>> +    size = sizeof(*virt) + count * sizeof(struct gpio_desc *);
>> +    virt = devm_kzalloc(dev, size, GFP_KERNEL);
> there's a helper now: size struct_size(virt, gpios, count), eg:
>     virt = devm_kzalloc(dev, struct_size(virt, gpios, count), GFP_KERNEL);

Didn't we also have a macro for allocating structs ?
(IIRC there at was least one for struct arrays)

>> +static int __init gpio_inverter_init(void)
>> +{
>> +    return platform_driver_register(&gpio_inverter_driver);
>> +}
>> +late_initcall(gpio_inverter_init);
>> +
>> +static void __exit gpio_inverter_exit(void)
>> +{
>> +    platform_driver_unregister(&gpio_inverter_driver);
>> +}

IMHO, we should introduce a macro for that, similar to
module_i2c_driver()+friends.


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@xxxxxxxxx -- +49-151-27565287