Re: [PATCH] gpio/sifive: fix static checker warning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



JaeJoon,

On 2020-01-29 01:27, JaeJoon Jung wrote:
Because SiFive FU540 GPIO Registers are aligned 32-bit,
I think that irq_state is good "unsigned int" than "unsigned long".

I refer to SiFive FU540-C000 Manual v1p0 (GPIO Register Table 103)
as "Only naturally aligned 32-bit memory accesses are supported"

You realize that we're talking about variables here, and not hardware
registers, right?

when you use assign_bit(offset, &chip->irq_state, 1),
offset is 32-bit.

And? assign_bit takes an "unsigned long *" as a parameter. irrespective
of the size of long on a given architecture, by the way.

I prefer to use bit operation instead of assign_bit().

u32 bit = BIT(offset);
chip->irq_state |= bit;

which is not what assign_bit() does.

If you use this code, you do not use the assign_bit() and
need not change irq_state data type.

Or we can use the correct API and not introduce additional bugs, which
your suggestion above would lead to.

There are many improvements in my works for drivers/gpio/gpio-sifive.c.
I hope to check my attached source file.

That's not how we submit patches to the Linux kernel. I suggest you
read the documentation on how to do this.

Thanks,

        M.
--
Jazz is not dead. It just smells funny...



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux