Re: [PATCH v2 2/2] gpio: Add Cadence GPIO driver

wt., 18 gru 2018 o 14:54 Janek Kotas [off-list ref] napisał(a):

quoted

On 18 Dec 2018, at 13:50, Bartosz Golaszewski [off-list ref] wrote:


pon., 17 gru 2018 o 23:22 Linus Walleij [off-list ref] napisał(a):

quoted

On Mon, Dec 17, 2018 at 4:51 PM Bartosz Golaszewski
[off-list ref] wrote:

quoted

The driver looks good but is there any particular reason not to use
regmap for register IO?

I thought we only use regmap for MMIO when the register range is
shared (as in a system controller) so that some registers are for this,
some register or even bits in a register for some other driver, so they
need the spinlock in the regmap to protect the register range.

This is what syscon is for. Regmap simply abstracts any register IO.
For instance: there's no locking in this driver. Are we sure it's not
needed? Regmap provides internal locking for you in the form of a
mutex or spinlock.

Also: it looks like the interrupts here are quite simple with a single
bit per interrupt in the status register and the same layout in the
mask register - it could probably profit from using the
regmap_irq_chip and not bother with reimplementing irq_chip callbacks.

quoted

It is also nice for shadowing/caching of register contents I guess,
wat does this driver get from regmap MMIO?

Code shrinkage IMO.

Note that I'm not blocking this from being merged - I just think that
using modern frameworks is always a good idea.

I can reimplement the driver using regmap, but It seems in such case
I won’t be able to use the Generic GPIO Infrastructure, would I?
So I would need to provide functions for setting direction, etc.
I think it would make the driver code bigger.

Indeed. If anything: gpio-mmio would need to be converted to using regmap.

So I guess nevermind my comment.

Bart