Hi Ming,
thanks for your patch!
Some nits below:
On Tue, Dec 10, 2024 at 11:46 AM Ming Yu <a0282524688@xxxxxxxxx> wrote:
> This driver supports GPIO and IRQ functionality for NCT6694 MFD
> device based on USB interface.
>
> Signed-off-by: Ming Yu <tmyu0@xxxxxxxxxxx>
(...)
> +#include <linux/gpio/driver.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/nct6694.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
#include <linux/bits.h>
is missing, include it explicitly.
> + return !(BIT(offset) & data->xmit_buf);
Here you use the BIT() macro from <linux/bits.h>
> +static int nct6694_direction_input(struct gpio_chip *gpio, unsigned int offset)
> +{
> + struct nct6694_gpio_data *data = gpiochip_get_data(gpio);
> + int ret;
> +
> + guard(mutex)(&data->lock);
> +
> + ret = nct6694_read_msg(data->nct6694, NCT6694_GPIO_MOD,
> + NCT6694_GPO_DIR + data->group,
> + NCT6694_GPIO_LEN, &data->xmit_buf);
> + if (ret < 0)
> + return ret;
> +
> + data->xmit_buf &= ~(1 << offset);
data->xmit_buf &= ~BIT(offset);
> +static int nct6694_direction_output(struct gpio_chip *gpio,
> + unsigned int offset, int val)
> +{
> + struct nct6694_gpio_data *data = gpiochip_get_data(gpio);
> + int ret;
> +
> + guard(mutex)(&data->lock);
> +
> + /* Set direction to output */
> + ret = nct6694_read_msg(data->nct6694, NCT6694_GPIO_MOD,
> + NCT6694_GPO_DIR + data->group,
> + NCT6694_GPIO_LEN, &data->xmit_buf);
> + if (ret < 0)
> + return ret;
> +
> + data->xmit_buf |= (1 << offset);
data->xmit_buf |= BIT(offset);
> + if (val)
> + data->xmit_buf |= (1 << offset);
> + else
> + data->xmit_buf &= ~(1 << offset);
Same
> +static void nct6694_set_value(struct gpio_chip *gpio, unsigned int offset,
> + int val)
> +{
(...)
> + if (val)
> + data->xmit_buf |= (1 << offset);
> + else
> + data->xmit_buf &= ~(1 << offset);
Same
> +static irqreturn_t nct6694_irq_handler(int irq, void *priv)
> +{
> + struct nct6694_gpio_data *data = priv;
> + unsigned char status;
> +
> + guard(mutex)(&data->lock);
> +
> + nct6694_read_msg(data->nct6694, NCT6694_GPIO_MOD,
> + NCT6694_GPI_STS + data->group,
> + NCT6694_GPIO_LEN, &data->xmit_buf);
> +
> + status = data->xmit_buf;
> +
> + while (status) {
> + int bit = __ffs(status);
> +
> + data->xmit_buf = BIT(bit);
> + handle_nested_irq(irq_find_mapping(data->gpio.irq.domain, bit));
> + status &= ~(1 << bit);
Same
Just use BIT() consistently please.
Yours,
Linus Walleij
