public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	David Laight <David.Laight@aculab.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 6/7] gpio: exar: switch to using regmap
Date: Tue, 10 Nov 2020 19:08:20 +0200	[thread overview]
Message-ID: <20201110170820.GC4077@smile.fi.intel.com> (raw)
In-Reply-To: <CAMRc=McyHfti_9ner8vyF=J=srzWJXsa7GRmwKF1TJ=ksGQM5g@mail.gmail.com>

On Tue, Nov 10, 2020 at 05:52:26PM +0100, Bartosz Golaszewski wrote:
> On Tue, Nov 10, 2020 at 5:43 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Tue, Nov 10, 2020 at 6:37 PM Bartosz Golaszewski
> > <bgolaszewski@baylibre.com> wrote:
> > > On Tue, Nov 10, 2020 at 5:17 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:

...

> > It's not a typo. But thinking again. This is basically done in regmap
> > to support serial buses. Here we have MMIO pretty much with 32-bit or
> > 64-bit address accesses. I didn't dig into regmap implementation to
> > understand the consequences of changing this to the different values
> > (it seems like rather offset, and in this case 11 is a correct one,
> > not a typo, and regmap is okay with that).
> > But I would rather ask Jan to actually mount debugfs and dump
> > registers and see if it screws up the UART (because it may go all over
> > important registers), that's why I think this configuration is still
> > missing some strict rules about what addresses (offsets) driver may or
> > may not access.
> 
> Ok now I get it. Yes 11 seems to be right in this case for the max
> address. We can implement the readable/writable callbacks to be very
> strict about the register accesses but isn't it overkill? This driver
> is very small and only accesses a couple registers. I don't see such
> strict checking very often except for very complicated modules (like
> pca953x you mentioned).

Maybe a comment in commit message or code that this has no protection against
access to out of boundary registers.

Keep my tag after choosing 11 and whatever you decide for access to non-GPIO
registers from this driver. I'm not blocking this from upstreaming since we
have got a confirmation that main functionality works as expected.


-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2020-11-10 17:07 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-10 14:55 [PATCH v4 0/7] gpio: exar: refactor the driver Bartosz Golaszewski
2020-11-10 14:55 ` [PATCH v4 1/7] gpio: exar: add a newline after the copyright notice Bartosz Golaszewski
2020-11-10 14:55 ` [PATCH v4 2/7] gpio: exar: include idr.h Bartosz Golaszewski
2020-11-10 14:55 ` [PATCH v4 3/7] gpio: exar: switch to a simpler IDA interface Bartosz Golaszewski
2020-11-10 14:55 ` [PATCH v4 4/7] gpio: exar: use a helper variable for &pdev->dev Bartosz Golaszewski
2020-11-10 14:55 ` [PATCH v4 5/7] gpio: exar: unduplicate address and offset computation Bartosz Golaszewski
2020-11-10 14:55 ` [PATCH v4 6/7] gpio: exar: switch to using regmap Bartosz Golaszewski
2020-11-10 15:04   ` Andy Shevchenko
2020-11-10 15:10     ` Andy Shevchenko
2020-11-10 15:12       ` Bartosz Golaszewski
2020-11-10 16:12         ` Andy Shevchenko
2020-11-10 16:36           ` Bartosz Golaszewski
2020-11-10 16:44             ` Andy Shevchenko
2020-11-10 16:52               ` Bartosz Golaszewski
2020-11-10 17:08                 ` Andy Shevchenko [this message]
2020-11-10 14:55 ` [PATCH v4 7/7] gpio: exar: use devm action for freeing the IDA and drop remove() Bartosz Golaszewski
2020-11-10 15:07 ` [PATCH v4 0/7] gpio: exar: refactor the driver Andy Shevchenko
2020-11-10 16:19   ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201110170820.GC4077@smile.fi.intel.com \
    --to=andy.shevchenko@gmail.com \
    --cc=David.Laight@aculab.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=brgl@bgdev.pl \
    --cc=jan.kiszka@siemens.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox