Re: [V2 5/5] powerpc: kernel: 16650 UART reg-shift support

On Wed, May 9, 2012 at 10:57 AM, Tanmay Inamdar [off-list ref] wrote:

On Wed, May 2, 2012 at 7:08 PM, Josh Boyer [off-list ref] wrote:

quoted

On Mon, Apr 9, 2012 at 3:20 AM, Tanmay Inamdar [off-list ref] wrote:

quoted

In APM8018X SOC, UART register address space has been relocated to 32-b=

it

quoted

quoted

data boundaries for APB bus implementation.
Current legacy_serial driver ignores the reg-shift property. This patch
modifies legacy_serial.c and udbg_16550.c to work with above mentioned =

UARTs.

quoted

quoted

Signed-off-by: Tanmay Inamdar <redacted>
---
:100644 100644 8338aef... f5fc106... M =A0arch/powerpc/include/asm/udbg=

.h

quoted

quoted

:100644 100644 bedd12e... d523b7d... M =A0arch/powerpc/kernel/legacy_se=

rial.c

quoted

quoted

:100644 100644 6837f83... e0cb7dc... M =A0arch/powerpc/kernel/udbg_1655=

0.c

quoted

quoted

=A0arch/powerpc/include/asm/udbg.h =A0 =A0 | =A0 =A02 +-
=A0arch/powerpc/kernel/legacy_serial.c | =A0 16 +++++---
=A0arch/powerpc/kernel/udbg_16550.c =A0 =A0| =A0 64 +++++++++++++++++++=

+++------------

quoted

quoted

=A03 files changed, 52 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/include/asm/udbg.h b/arch/powerpc/include/asm=

/udbg.h

quoted

quoted

index 8338aef..f5fc106 100644

--- a/arch/powerpc/include/asm/udbg.h
+++ b/arch/powerpc/include/asm/udbg.h

@@ -29,7 +29,7 @@ extern void udbg_printf(const char *fmt, ...)

=A0extern void udbg_progress(char *s, unsigned short hex);

=A0extern void udbg_init_uart(void __iomem *comport, unsigned int speed=

,

quoted

quoted

- =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0unsigned int clock=

);

quoted

quoted

+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0unsigned int clock=

, =A0unsigned int regshift);

quoted

quoted

=A0extern unsigned int udbg_probe_uart_speed(void __iomem *comport,
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=

 =A0 =A0 =A0unsigned int clock);

quoted

quoted

diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/=

legacy_serial.c

quoted

quoted

index bedd12e..d523b7d 100644

--- a/arch/powerpc/kernel/legacy_serial.c
+++ b/arch/powerpc/kernel/legacy_serial.c

@@ -33,6 +33,7 @@ static struct legacy_serial_info {

=A0 =A0 =A0 =A0unsigned int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0cloc=

k;

quoted

quoted

=A0 =A0 =A0 =A0int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

=A0 irq_check_parent;

quoted

quoted

=A0 =A0 =A0 =A0phys_addr_t =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 tadd=

r;

quoted

quoted

+ =A0 =A0 =A0 unsigned int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0regsh=

ift;

quoted

quoted

=A0} legacy_serial_infos[MAX_LEGACY_SERIAL_PORTS];

=A0static struct __initdata of_device_id legacy_serial_parents[] =3D {

@@ -42,6 +43,7 @@ static struct __initdata of_device_id legacy_serial_p=

arents[] =3D {

quoted

quoted

=A0 =A0 =A0 =A0{.compatible =3D "ibm,opb",},
=A0 =A0 =A0 =A0{.compatible =3D "simple-bus",},
=A0 =A0 =A0 =A0{.compatible =3D "wrs,epld-localbus",},
+ =A0 =A0 =A0 {.compatible =3D "apm,apb",},
=A0 =A0 =A0 =A0{},
=A0};

@@ -163,11 +165,6 @@ static int __init add_legacy_soc_port(struct devic=

e_node *np,

quoted

quoted

=A0 =A0 =A0 =A0if (of_get_property(np, "clock-frequency", NULL) =3D=3D =

NULL)

quoted

quoted

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -1;

- =A0 =A0 =A0 /* if reg-shift or offset, don't try to use it */
- =A0 =A0 =A0 if ((of_get_property(np, "reg-shift", NULL) !=3D NULL) ||
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 (of_get_property(np, "reg-offset", NULL) =

!=3D NULL))

quoted

quoted

- =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -1;
-

So we explicitly didn't support reg-shift before. =A0I'm guessing there =

is

quoted

a reason for that, but I don't recall what. =A0Ben?

Also, why do you need to use the legacy serial driver at all for this
SOC? =A0As far as I remember, the OF serial driver should be sufficient.

You are right. There is no need to use legacy serial driver. However I
realized that when 40x is selected, 'PPC_UDBG_16550' is by default
selected. This enables legacy serial driver.

Is it now required to enable 'PPC_UDBG_16550' by default for every SOC
that uses 40x processor?

Josh, Ben,

Please let me know if you have any comments regarding above question.

quoted

quoted

+static unsigned int reg_shift;
+#define ns16550_offset(addr) (addr - (unsigned char *)udbg_comport)
+
=A0static struct NS16550 __iomem *udbg_comport;

+static inline u8 serial_read(unsigned char *addr)
+{
+ =A0 =A0 =A0 u32 offset =3D ns16550_offset(addr) << reg_shift;
+ =A0 =A0 =A0 return readb(udbg_comport + offset);
+}
+
+static inline void serial_write(unsigned char *addr, char val)
+{
+ =A0 =A0 =A0 u32 offset =3D ns16550_offset(addr) << reg_shift;
+ =A0 =A0 =A0 writeb(val, udbg_comport + offset);
+}
+

I don't think readb/writeb are correct here. =A0Why did you switch to
using those instead of sticking with in_8/out_8?

josh

Thanks,
Tanmay

CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, =

is for the sole use of the intended recipient(s) and contains information=
=A0
that is confidential and proprietary to AppliedMicro Corporation or its s=
ubsidiaries. =

It is to be used solely for the purpose of furthering the parties' busine=
ss relationship. =

All unauthorized review, use, disclosure or distribution is prohibited. =

If you are not the intended recipient, please contact the sender by reply=
 e-mail =

and destroy all copies of the original message.
=0D