regmap-mmio and paged registers

regmap-mmio and paged registers

[Alexandre Belloni]

Hi Mark,

On AT91, we have a paged register inside a register range.

This register looks like that:

AT91_PMC_PCR:
   31   30   29   28   27   26   25   24
 +----+----+----+----+----+----+----+----+
 | -- | -- | -- | EN | -- | -- | -- | -- |
 +----+----+----+----+----+----+----+----+

   23   22   21   20   19   18   17   16
 +----+----+----+----+----+----+----+----+
 | -- | -- | -- | -- | -- | -- |   DIV   |
 +----+----+----+----+----+----+----+----+

   15   14   13   12   11   10   09   08
 +----+----+----+----+----+----+----+----+
 | -- | -- | -- |CMD | -- | -- | -- | -- |
 +----+----+----+----+----+----+----+----+

   07   06   05   04   03   02   01   00
 +----+----+----+----+----+----+----+----+
 | -- | -- |             PID             |
 +----+----+----+----+----+----+----+----+

PID is our selector, the other bits are the data.

We described the regmap by creating a virtual range:

#define AT91_PMC_VIRT_PCR(id)	(0x200 + id)

static const struct regmap_range_cfg pmc_regmap_ranges[] = {
	{
		.range_min = AT91_PMC_VIRT_PCR(0),
		.range_max = AT91_PMC_VIRT_PCR(AT91_PMC_PCR_PID_MASK),
		.window_start = AT91_PMC_PCR,
		.window_len = 1,
		.selector_reg = AT91_PMC_PCR,
		.selector_mask = 0xffffffff,
	},
};

static struct regmap_config at91sam9x5_pmc_regmap_config = {
	.reg_bits = 32,
	.val_bits = 32,
	.reg_stride = 4,
	.ranges = pmc_regmap_ranges,
	.num_ranges = ARRAY_SIZE(pmc_regmap_ranges),
	.max_register = AT91_PMC_VIRT_PCR(AT91_PMC_PCR_PID_MASK),
};

The issue we have with that setup is that _regmap_select_page() does
that check before writing the page number:

	/* It is possible to have selector register inside data window.
	   In that case, selector register is located on every page and
	   it needs no page switching, when accessed alone. */
	if (val_num > 1 ||
	    range->window_start + win_offset != range->selector_reg) {

So it ends up never writing the page number in the register.

One possible solution would be to implement our own .read and .write to
handle that paging but maybe you can think of something else.


The full datasheet is here, 27.16.26 PMC Peripheral Control Register,
page 270
http://www.atmel.com/Images/Atmel-11121-32-bit-Cortex-A5-Microcontroller-SAMA5D3_Datasheet.pdf

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

regmap-mmio and paged registers

[Mark Brown]

On Wed, Dec 09, 2015 at 10:41:50PM +0100, Alexandre Belloni wrote:

> 	/* It is possible to have selector register inside data window.
> 	   In that case, selector register is located on every page and
> 	   it needs no page switching, when accessed alone. */
> 	if (val_num > 1 ||
> 	    range->window_start + win_offset != range->selector_reg) {

> So it ends up never writing the page number in the register.

> One possible solution would be to implement our own .read and .write to
> handle that paging but maybe you can think of something else.

So to be explicit about the actual issue the problem here is that you
have data bits in the same register as your selector and indeed the
register map you're trying to page is a single register.  That's pretty
pathological.

Honestly looking at the register I'm not even convinced it is a paged
regmap, it looks more like a mailbox for communication with a
coprocessor than anything else (the fact that the selector is named PID,
the fact that the bitfields include CMD...).  Are you sure that it's a
good idea to represent this as a regmap at all?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151209/7d65c2bd/attachment.sig>

regmap-mmio and paged registers

[Alexandre Belloni]

On 09/12/2015 at 22:10:36 +0000, Mark Brown wrote :
> On Wed, Dec 09, 2015 at 10:41:50PM +0100, Alexandre Belloni wrote:
> 
> > 	/* It is possible to have selector register inside data window.
> > 	   In that case, selector register is located on every page and
> > 	   it needs no page switching, when accessed alone. */
> > 	if (val_num > 1 ||
> > 	    range->window_start + win_offset != range->selector_reg) {
> 
> > So it ends up never writing the page number in the register.
> 
> > One possible solution would be to implement our own .read and .write to
> > handle that paging but maybe you can think of something else.
> 
> So to be explicit about the actual issue the problem here is that you
> have data bits in the same register as your selector and indeed the
> register map you're trying to page is a single register.  That's pretty
> pathological.
> 
> Honestly looking at the register I'm not even convinced it is a paged
> regmap, it looks more like a mailbox for communication with a
> coprocessor than anything else (the fact that the selector is named PID,
> the fact that the bitfields include CMD...).  Are you sure that it's a
> good idea to represent this as a regmap at all?

Yeah, I was probably not clear enough but the regmap actually covers the
whole PMC range and include that particular register. We have to do that
because this range is used by multiple drivers.

This register, PMC_PCR is also used by multiple drivers so it is
important to properly lock between the page selection and the following
read or write.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

regmap-mmio and paged registers

[Mark Brown]

On Wed, Dec 09, 2015 at 11:26:13PM +0100, Alexandre Belloni wrote:
> On 09/12/2015 at 22:10:36 +0000, Mark Brown wrote :

> > Honestly looking at the register I'm not even convinced it is a paged
> > regmap, it looks more like a mailbox for communication with a
> > coprocessor than anything else (the fact that the selector is named PID,
> > the fact that the bitfields include CMD...).  Are you sure that it's a
> > good idea to represent this as a regmap at all?

> Yeah, I was probably not clear enough but the regmap actually covers the
> whole PMC range and include that particular register. We have to do that
> because this range is used by multiple drivers.

I don't see how that follows?  You can layer anything you like on top of
a regmap.

> This register, PMC_PCR is also used by multiple drivers so it is
> important to properly lock between the page selection and the following
> read or write.

regmaps aren't the only way of sharing access to something - like I say
there's the mailbox API too.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151216/cb6cfecd/attachment.sig>