[U-Boot] [RFC] ns16550: Add support for AUX regs usage on some ARC SoCs

[U-Boot] [RFC] ns16550: Add support for AUX regs usage on some ARC SoCs

[Alexey Brodkin]

Synopsys Data Fusion subsystem (DFSS) is targeted to deeply built-in
use-cases and so to save some silicon area decision was made to
escape usage of any busses and use instead directly wired to CPU
peripherals. And one of those is DW APB UART.

Later DFSS became a part of larger and more complicated SoCs with
some other peripherals connected via common buses but default UART
is still used via ARC core's auxulary registers which are not mapped
to "normal" address space and we use very special instructions to access
them, thus we cannot simply reuse whatever accessor we have in 16550
UART as of now.

Also we cannot just switch inb()/outb() to access ARC AUX regs always
for DFSS because other peripherals have normal memory-mapped control
registers and we need to use normal accessors for them.

Frankly I don't like a lot what I did here but otherwise if I create
a special driver for this I'll need to reimplement
ns16550_serial_ops.putc()/getc() which will be pure copy-paseted from
ns16550.c because the only difference is only in
ns16550_{read|write}b().

As mentioned above we cannot remap those auxiliary registers to
normal memory address-space and thus we have to use very special
accessors write_aux_reg()/read_aux_reg() that directly use special
CPU intructions (namely LR/SR) for dealing with ARC AUX regs.

Also note here I just use a check for a particular SoC being selected
(CONFIG_ARCH_DFSS will be introduced shortly) but that is done just for
simplicity, otherwise it might be a slecial Kconfig option for NS16550
or anything else.

I'd like to know what people think about possible colutions here.
And as always any comments are much appreciated!

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: Stefan Roese <sr@denx.de>
---
 drivers/serial/ns16550.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index 6f9ce689cfff..75f342f337f1 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -13,6 +13,9 @@
 #include <watchdog.h>
 #include <linux/types.h>
 #include <asm/io.h>
+#ifdef CONFIG_ARCH_DFSS
+#include <asm/arcregs.h>
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -53,7 +56,9 @@ DECLARE_GLOBAL_DATA_PTR;
 
 static inline void serial_out_shift(void *addr, int shift, int value)
 {
-#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
+#ifdef CONFIG_ARCH_DFSS
+	write_aux_reg((int)addr, value);
+#elif defined(CONFIG_SYS_NS16550_PORT_MAPPED)
 	outb(value, (ulong)addr);
 #elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN)
 	out_le32(addr, value);
@@ -70,7 +75,9 @@ static inline void serial_out_shift(void *addr, int shift, int value)
 
 static inline int serial_in_shift(void *addr, int shift)
 {
-#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
+#ifdef CONFIG_ARCH_DFSS
+	return read_aux_reg((int)addr);
+#elif defined(CONFIG_SYS_NS16550_PORT_MAPPED)
 	return inb((ulong)addr);
 #elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN)
 	return in_le32(addr);
-- 
2.14.3

[U-Boot] [RFC] ns16550: Add support for AUX regs usage on some ARC SoCs

[Tom Rini]

On Wed, Feb 21, 2018 at 03:26:05PM +0300, Alexey Brodkin wrote:
> Synopsys Data Fusion subsystem (DFSS) is targeted to deeply built-in
> use-cases and so to save some silicon area decision was made to
> escape usage of any busses and use instead directly wired to CPU
> peripherals. And one of those is DW APB UART.
> 
> Later DFSS became a part of larger and more complicated SoCs with
> some other peripherals connected via common buses but default UART
> is still used via ARC core's auxulary registers which are not mapped
> to "normal" address space and we use very special instructions to access
> them, thus we cannot simply reuse whatever accessor we have in 16550
> UART as of now.
> 
> Also we cannot just switch inb()/outb() to access ARC AUX regs always
> for DFSS because other peripherals have normal memory-mapped control
> registers and we need to use normal accessors for them.
> 
> Frankly I don't like a lot what I did here but otherwise if I create
> a special driver for this I'll need to reimplement
> ns16550_serial_ops.putc()/getc() which will be pure copy-paseted from
> ns16550.c because the only difference is only in
> ns16550_{read|write}b().
> 
> As mentioned above we cannot remap those auxiliary registers to
> normal memory address-space and thus we have to use very special
> accessors write_aux_reg()/read_aux_reg() that directly use special
> CPU intructions (namely LR/SR) for dealing with ARC AUX regs.
> 
> Also note here I just use a check for a particular SoC being selected
> (CONFIG_ARCH_DFSS will be introduced shortly) but that is done just for
> simplicity, otherwise it might be a slecial Kconfig option for NS16550
> or anything else.
> 
> I'd like to know what people think about possible colutions here.
> And as always any comments are much appreciated!
> 
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Stefan Roese <sr@denx.de>
> ---
>  drivers/serial/ns16550.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index 6f9ce689cfff..75f342f337f1 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -13,6 +13,9 @@
>  #include <watchdog.h>
>  #include <linux/types.h>
>  #include <asm/io.h>
> +#ifdef CONFIG_ARCH_DFSS
> +#include <asm/arcregs.h>
> +#endif
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -53,7 +56,9 @@ DECLARE_GLOBAL_DATA_PTR;
>  
>  static inline void serial_out_shift(void *addr, int shift, int value)
>  {
> -#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> +#ifdef CONFIG_ARCH_DFSS
> +	write_aux_reg((int)addr, value);
> +#elif defined(CONFIG_SYS_NS16550_PORT_MAPPED)
>  	outb(value, (ulong)addr);
>  #elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN)
>  	out_le32(addr, value);
> @@ -70,7 +75,9 @@ static inline void serial_out_shift(void *addr, int shift, int value)
>  
>  static inline int serial_in_shift(void *addr, int shift)
>  {
> -#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> +#ifdef CONFIG_ARCH_DFSS
> +	return read_aux_reg((int)addr);
> +#elif defined(CONFIG_SYS_NS16550_PORT_MAPPED)
>  	return inb((ulong)addr);
>  #elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN)
>  	return in_le32(addr);

As always, thanks for the detailed explanation.  Yes, I think that of
the options, putting the details in read/write_aux_reg (and please make
sure read/write_aux_reg have a good function comment too) is the best
choice.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180222/c738c4cb/attachment.sig>

[U-Boot] [RFC] ns16550: Add support for AUX regs usage on some ARC SoCs

[Alexey Brodkin]

Hi Tom,

On Thu, 2018-02-22 at 10:43 -0500, Tom Rini wrote:
> On Wed, Feb 21, 2018 at 03:26:05PM +0300, Alexey Brodkin wrote:

[snip]

> >  static inline void serial_out_shift(void *addr, int shift, int value)
> >  {
> > -#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> > +#ifdef CONFIG_ARCH_DFSS
> > +	write_aux_reg((int)addr, value);
> > +#elif defined(CONFIG_SYS_NS16550_PORT_MAPPED)
> >  	outb(value, (ulong)addr);
> >  #elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN)
> >  	out_le32(addr, value);
> > @@ -70,7 +75,9 @@ static inline void serial_out_shift(void *addr, int shift, int value)
> >  
> >  static inline int serial_in_shift(void *addr, int shift)
> >  {
> > -#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> > +#ifdef CONFIG_ARCH_DFSS
> > +	return read_aux_reg((int)addr);
> > +#elif defined(CONFIG_SYS_NS16550_PORT_MAPPED)
> >  	return inb((ulong)addr);
> >  #elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN)
> >  	return in_le32(addr);
> 
> As always, thanks for the detailed explanation.  Yes, I think that of
> the options, putting the details in read/write_aux_reg (and please make
> sure read/write_aux_reg have a good function comment too) is the best
> choice.  Thanks!

Frankly I didn't understand your comment well enough :)
Do you suggest to keep proposed implementation (i.e. modification of
serial_{in|out}_shift()) but add comments on what do we do and why or
you really meant something completely different?

-Alexey

[U-Boot] [RFC] ns16550: Add support for AUX regs usage on some ARC SoCs

[Simon Glass]

Hi,

On 21 February 2018 at 05:26, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:
> Synopsys Data Fusion subsystem (DFSS) is targeted to deeply built-in
> use-cases and so to save some silicon area decision was made to
> escape usage of any busses and use instead directly wired to CPU
> peripherals. And one of those is DW APB UART.
>
> Later DFSS became a part of larger and more complicated SoCs with
> some other peripherals connected via common buses but default UART
> is still used via ARC core's auxulary registers which are not mapped
> to "normal" address space and we use very special instructions to access
> them, thus we cannot simply reuse whatever accessor we have in 16550
> UART as of now.
>
> Also we cannot just switch inb()/outb() to access ARC AUX regs always
> for DFSS because other peripherals have normal memory-mapped control
> registers and we need to use normal accessors for them.
>
> Frankly I don't like a lot what I did here but otherwise if I create
> a special driver for this I'll need to reimplement
> ns16550_serial_ops.putc()/getc() which will be pure copy-paseted from
> ns16550.c because the only difference is only in
> ns16550_{read|write}b().
>
> As mentioned above we cannot remap those auxiliary registers to
> normal memory address-space and thus we have to use very special
> accessors write_aux_reg()/read_aux_reg() that directly use special
> CPU intructions (namely LR/SR) for dealing with ARC AUX regs.
>
> Also note here I just use a check for a particular SoC being selected
> (CONFIG_ARCH_DFSS will be introduced shortly) but that is done just for
> simplicity, otherwise it might be a slecial Kconfig option for NS16550
> or anything else.
>
> I'd like to know what people think about possible colutions here.
> And as always any comments are much appreciated!
>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Stefan Roese <sr@denx.de>
> ---
>  drivers/serial/ns16550.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)

I think a separate driver might be better, unless we want to make the
read/write interface go through regmap or similar?

Regards,
Simon

[U-Boot] [RFC] ns16550: Add support for AUX regs usage on some ARC SoCs

[Tom Rini]

On Thu, Feb 22, 2018 at 04:07:39PM +0000, Alexey Brodkin wrote:
> Hi Tom,
> 
> On Thu, 2018-02-22 at 10:43 -0500, Tom Rini wrote:
> > On Wed, Feb 21, 2018 at 03:26:05PM +0300, Alexey Brodkin wrote:
> 
> [snip]
> 
> > >  static inline void serial_out_shift(void *addr, int shift, int value)
> > >  {
> > > -#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> > > +#ifdef CONFIG_ARCH_DFSS
> > > +	write_aux_reg((int)addr, value);
> > > +#elif defined(CONFIG_SYS_NS16550_PORT_MAPPED)
> > >  	outb(value, (ulong)addr);
> > >  #elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN)
> > >  	out_le32(addr, value);
> > > @@ -70,7 +75,9 @@ static inline void serial_out_shift(void *addr, int shift, int value)
> > >  
> > >  static inline int serial_in_shift(void *addr, int shift)
> > >  {
> > > -#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> > > +#ifdef CONFIG_ARCH_DFSS
> > > +	return read_aux_reg((int)addr);
> > > +#elif defined(CONFIG_SYS_NS16550_PORT_MAPPED)
> > >  	return inb((ulong)addr);
> > >  #elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN)
> > >  	return in_le32(addr);
> > 
> > As always, thanks for the detailed explanation.  Yes, I think that of
> > the options, putting the details in read/write_aux_reg (and please make
> > sure read/write_aux_reg have a good function comment too) is the best
> > choice.  Thanks!
> 
> Frankly I didn't understand your comment well enough :)
> Do you suggest to keep proposed implementation (i.e. modification of
> serial_{in|out}_shift()) but add comments on what do we do and why or
> you really meant something completely different?

I'm fine with what you have above, in the driver.  I missed that
read_aux_reg already exists, sorry.  So I guess a detailed commit
message will have to be enough documentation for future reviewers of the
code.  Or, we'll see what comes out of Simon's comments.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180222/c6648a89/attachment.sig>

[U-Boot] [RFC] ns16550: Add support for AUX regs usage on some ARC SoCs

[Alexey Brodkin]

Hi Simon,

On Thu, 2018-02-22 at 09:17 -0700, Simon Glass wrote:
> Hi,
> 
> On 21 February 2018 at 05:26, Alexey Brodkin
> <Alexey.Brodkin@synopsys.com> wrote:
> > Synopsys Data Fusion subsystem (DFSS) is targeted to deeply built-in
> > use-cases and so to save some silicon area decision was made to
> > escape usage of any busses and use instead directly wired to CPU
> > peripherals. And one of those is DW APB UART.
> > 
> > Later DFSS became a part of larger and more complicated SoCs with
> > some other peripherals connected via common buses but default UART
> > is still used via ARC core's auxulary registers which are not mapped
> > to "normal" address space and we use very special instructions to access
> > them, thus we cannot simply reuse whatever accessor we have in 16550
> > UART as of now.
> > 
> > Also we cannot just switch inb()/outb() to access ARC AUX regs always
> > for DFSS because other peripherals have normal memory-mapped control
> > registers and we need to use normal accessors for them.
> > 
> > Frankly I don't like a lot what I did here but otherwise if I create
> > a special driver for this I'll need to reimplement
> > ns16550_serial_ops.putc()/getc() which will be pure copy-paseted from
> > ns16550.c because the only difference is only in
> > ns16550_{read|write}b().
> > 
> > As mentioned above we cannot remap those auxiliary registers to
> > normal memory address-space and thus we have to use very special
> > accessors write_aux_reg()/read_aux_reg() that directly use special
> > CPU intructions (namely LR/SR) for dealing with ARC AUX regs.
> > 
> > Also note here I just use a check for a particular SoC being selected
> > (CONFIG_ARCH_DFSS will be introduced shortly) but that is done just for
> > simplicity, otherwise it might be a slecial Kconfig option for NS16550
> > or anything else.
> > 
> > I'd like to know what people think about possible colutions here.
> > And as always any comments are much appreciated!
> > 
> > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: Stefan Roese <sr@denx.de>
> > ---
> >  drivers/serial/ns16550.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> I think a separate driver might be better, unless we want to make the
> read/write interface go through regmap or similar?

But in case of ARC's AUX regs portmap won't help because those AUX regs are
couldn't be mapped - that a completely different address space and we may
only access them via dedicated instructions (LR vs LD and SR vs ST).

-Alexey

[U-Boot] [RFC] ns16550: Add support for AUX regs usage on some ARC SoCs

[Simon Glass]

Hi Alexey,

On 22 February 2018 at 09:23, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:
> Hi Simon,
>
> On Thu, 2018-02-22 at 09:17 -0700, Simon Glass wrote:
>> Hi,
>>
>> On 21 February 2018 at 05:26, Alexey Brodkin
>> <Alexey.Brodkin@synopsys.com> wrote:
>> > Synopsys Data Fusion subsystem (DFSS) is targeted to deeply built-in
>> > use-cases and so to save some silicon area decision was made to
>> > escape usage of any busses and use instead directly wired to CPU
>> > peripherals. And one of those is DW APB UART.
>> >
>> > Later DFSS became a part of larger and more complicated SoCs with
>> > some other peripherals connected via common buses but default UART
>> > is still used via ARC core's auxulary registers which are not mapped
>> > to "normal" address space and we use very special instructions to access
>> > them, thus we cannot simply reuse whatever accessor we have in 16550
>> > UART as of now.
>> >
>> > Also we cannot just switch inb()/outb() to access ARC AUX regs always
>> > for DFSS because other peripherals have normal memory-mapped control
>> > registers and we need to use normal accessors for them.
>> >
>> > Frankly I don't like a lot what I did here but otherwise if I create
>> > a special driver for this I'll need to reimplement
>> > ns16550_serial_ops.putc()/getc() which will be pure copy-paseted from
>> > ns16550.c because the only difference is only in
>> > ns16550_{read|write}b().
>> >
>> > As mentioned above we cannot remap those auxiliary registers to
>> > normal memory address-space and thus we have to use very special
>> > accessors write_aux_reg()/read_aux_reg() that directly use special
>> > CPU intructions (namely LR/SR) for dealing with ARC AUX regs.
>> >
>> > Also note here I just use a check for a particular SoC being selected
>> > (CONFIG_ARCH_DFSS will be introduced shortly) but that is done just for
>> > simplicity, otherwise it might be a slecial Kconfig option for NS16550
>> > or anything else.
>> >
>> > I'd like to know what people think about possible colutions here.
>> > And as always any comments are much appreciated!
>> >
>> > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
>> > Cc: Simon Glass <sjg@chromium.org>
>> > Cc: Tom Rini <trini@konsulko.com>
>> > Cc: Stefan Roese <sr@denx.de>
>> > ---
>> >  drivers/serial/ns16550.c | 11 +++++++++--
>> >  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> I think a separate driver might be better, unless we want to make the
>> read/write interface go through regmap or similar?
>
> But in case of ARC's AUX regs portmap won't help because those AUX regs are
> couldn't be mapped - that a completely different address space and we may
> only access them via dedicated instructions (LR vs LD and SR vs ST).

Well...

1. With a separate driver, you can do whatever you want :-) I know it
introduces code duplication though...

2. With regmap you can add your own regmap driver, and again do
whatever you want. I can help with that if it sounds attractive

Regards,
Simon

[U-Boot] [RFC] ns16550: Add support for AUX regs usage on some ARC SoCs

[Alexey Brodkin]

Hi Simon,

On Thu, 2018-02-22 at 10:29 -0700, Simon Glass wrote:
> Hi Alexey,
> 
> On 22 February 2018 at 09:23, Alexey Brodkin
> <Alexey.Brodkin@synopsys.com> wrote:
> > Hi Simon,
> > 
> > On Thu, 2018-02-22 at 09:17 -0700, Simon Glass wrote:
> > > Hi,
> > > 
> > > On 21 February 2018 at 05:26, Alexey Brodkin
> > > <Alexey.Brodkin@synopsys.com> wrote:

[snip]

> > > > ---
> > > >  drivers/serial/ns16550.c | 11 +++++++++--
> > > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > I think a separate driver might be better, unless we want to make the
> > > read/write interface go through regmap or similar?
> > 
> > But in case of ARC's AUX regs portmap won't help because those AUX regs are
> > couldn't be mapped - that a completely different address space and we may
> > only access them via dedicated instructions (LR vs LD and SR vs ST).
> 
> Well...
> 
> 1. With a separate driver, you can do whatever you want :-) I know it
> introduces code duplication though...
> 
> 2. With regmap you can add your own regmap driver, and again do
> whatever you want. I can help with that if it sounds attractive

That sounds definitely interesting!
Are there any pointers on how that usually done?

-Alexey

[U-Boot] [RFC] ns16550: Add support for AUX regs usage on some ARC SoCs

[Simon Glass]

Hi Alexey,

On 23 February 2018 at 11:08, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:
> Hi Simon,
>
> On Thu, 2018-02-22 at 10:29 -0700, Simon Glass wrote:
>> Hi Alexey,
>>
>> On 22 February 2018 at 09:23, Alexey Brodkin
>> <Alexey.Brodkin@synopsys.com> wrote:
>> > Hi Simon,
>> >
>> > On Thu, 2018-02-22 at 09:17 -0700, Simon Glass wrote:
>> > > Hi,
>> > >
>> > > On 21 February 2018 at 05:26, Alexey Brodkin
>> > > <Alexey.Brodkin@synopsys.com> wrote:
>
> [snip]
>
>> > > > ---
>> > > >  drivers/serial/ns16550.c | 11 +++++++++--
>> > > >  1 file changed, 9 insertions(+), 2 deletions(-)
>> > >
>> > > I think a separate driver might be better, unless we want to make the
>> > > read/write interface go through regmap or similar?
>> >
>> > But in case of ARC's AUX regs portmap won't help because those AUX regs are
>> > couldn't be mapped - that a completely different address space and we may
>> > only access them via dedicated instructions (LR vs LD and SR vs ST).
>>
>> Well...
>>
>> 1. With a separate driver, you can do whatever you want :-) I know it
>> introduces code duplication though...
>>
>> 2. With regmap you can add your own regmap driver, and again do
>> whatever you want. I can help with that if it sounds attractive
>
> That sounds definitely interesting!
> Are there any pointers on how that usually done?

Not yet. My ideas:

- Add a new regmap_init_xxx() function
- Adjust the regmap structure to allow read()/write() function
pointers to be provided
- Adjust the regmap code to call them, instead of the hard-coded
memory-mapped ones
- Adjust ns16550 to call regmap instead of readl/writel

This should clean up the mess in ns16550 a bit. But it will require a
bit of investigation / thinking.

Regards,
Simon

[U-Boot] [RFC] ns16550: Add support for AUX regs usage on some ARC SoCs

[Alexey Brodkin]

Hi Simon,

On Thu, 2018-02-22 at 10:29 -0700, Simon Glass wrote:
> Hi Alexey,
> 
> On 22 February 2018 at 09:23, Alexey Brodkin
> <Alexey.Brodkin@synopsys.com> wrote:
> > Hi Simon,
> > 
> > On Thu, 2018-02-22 at 09:17 -0700, Simon Glass wrote:

[snip]

> > > I think a separate driver might be better, unless we want to make the
> > > read/write interface go through regmap or similar?
> > 
> > But in case of ARC's AUX regs portmap won't help because those AUX regs are
> > couldn't be mapped - that a completely different address space and we may
> > only access them via dedicated instructions (LR vs LD and SR vs ST).
> 
> Well...
> 
> 1. With a separate driver, you can do whatever you want :-) I know it
> introduces code duplication though...

Exactly I hate to introduce another driver which will be 99,9% the same
as an existing one and then we'll need to care of it as well.

> 2. With regmap you can add your own regmap driver, and again do
> whatever you want. I can help with that if it sounds attractive

Ok so I took a look at regmap in Linux kernel and indeed that
will solve our problem: we'll have a regmap-mmio.c and regmap-arcaux.c
with appropriate implementation of accessors but I'm not really sure
it worth the trouble. Or your idea was to move all the different #ifdefs from
serial_{in|out}_shift() to the corresponding regmap implementations such that
we have something like below:
 regmap-mem32-le.c
 regmap-mem32-be.c
 regmap-portmapped.c
 etc
?

Regards,
Alexey

[U-Boot] [RFC] ns16550: Add support for AUX regs usage on some ARC SoCs

[Simon Glass]

Hi Alexey.

On 2 March 2018 at 14:51, Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote:
>
> Hi Simon,
>
> On Thu, 2018-02-22 at 10:29 -0700, Simon Glass wrote:
> > Hi Alexey,
> >
> > On 22 February 2018 at 09:23, Alexey Brodkin
> > <Alexey.Brodkin@synopsys.com> wrote:
> > > Hi Simon,
> > >
> > > On Thu, 2018-02-22 at 09:17 -0700, Simon Glass wrote:
>
> [snip]
>
> > > > I think a separate driver might be better, unless we want to make the
> > > > read/write interface go through regmap or similar?
> > >
> > > But in case of ARC's AUX regs portmap won't help because those AUX regs are
> > > couldn't be mapped - that a completely different address space and we may
> > > only access them via dedicated instructions (LR vs LD and SR vs ST).
> >
> > Well...
> >
> > 1. With a separate driver, you can do whatever you want :-) I know it
> > introduces code duplication though...
>
> Exactly I hate to introduce another driver which will be 99,9% the same
> as an existing one and then we'll need to care of it as well.
>
> > 2. With regmap you can add your own regmap driver, and again do
> > whatever you want. I can help with that if it sounds attractive
>
> Ok so I took a look at regmap in Linux kernel and indeed that
> will solve our problem: we'll have a regmap-mmio.c and regmap-arcaux.c
> with appropriate implementation of accessors but I'm not really sure
> it worth the trouble. Or your idea was to move all the different #ifdefs from
> serial_{in|out}_shift() to the corresponding regmap implementations such that
> we have something like below:
>  regmap-mem32-le.c
>  regmap-mem32-be.c
>  regmap-portmapped.c
>  etc
> ?

Yes, that's it.

I'm not sure why that driver is so special. Presumably other drivers
would have the same problem?

Regards,
Simon