* RE: [PATCH V1] serial: imx: revert setup DCEDTE early and ensure DCD and RI irqs to be off
From: Steve Twiss @ 2017-05-24 10:32 UTC (permalink / raw)
To: Fabio Estevam
Cc: Greg Kroah-Hartman, Jiri Slaby, LINUX-KERNEL, LINUX-SERIAL,
Lucas Stach, Uwe Kleine-Konig, Support Opensource
In-Reply-To: <CAOMZO5CT5MS1WGm_rCJ61Ambh+rMapsQWD84RXtagtt3HhmZwg@mail.gmail.com>
Hi Fabio,
On 23 May 2017 17:26 Fabio Estevam wrote:
> Subject: Re: [PATCH V1] serial: imx: revert setup DCEDTE early and ensure DCD and RI irqs to be off
> On Tue, May 23, 2017 at 9:17 AM, Steve Twiss wrote:
> >
> > Revert the commit e61c38d85b7392e ("serial: imx: setup DCEDTE early and
> > ensure DCD and RI irqs to be off")
> >
> > The patch submitted to setup DCEDTE early and ensure DCD and RI irqs to
> > be off, causes a serial console display problem the i.MX6Q SABRESD board.
> > The console becomes unreadable and unwritable.
> >
> > Tested-by: Steve Twiss <stwiss.opensource@diasemi.com>
> > Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
> >
> > ---
> > This patch applies against linux-next and v4.12-rc2
> >
> > Hi,
> >
> > I have been seeing a problem with the serial output console on the i.MX6Q
> > SABRESD, but not the i.MX6DL SABRESD. Everything was fine up to
> > linux-mainline/v4.11 but changed after linux-next/next-20170501.
> >
> > Some bisection has pointed at the commit
> > e61c38d85b7392e033ee03bca46f1d6006156175 which, once removed from my
> > linux-next/v4.12-rc2 build allows the i.MX6Q board to display the console
> > correctly again.
> >
> > This patch removes the original commit e61c38d85b7392e ("serial: imx:
> > setup DCEDTE early and ensure DCD and RI irqs to be off") from linux-next
> > v4.12-rc2 and fixes the serial problem seen in the i.MX6Q SABRESD board.
>
> How can the error be reproduced?
>
> Care to share more details of the error, please?
The USB to UART connection gets corrupted.
If this patch is applied to the kernel, the i.MX6 Q (quad), and only this board as far as
we know, starts to fail. This does *not* change the i.MX6DL and other sabre boards
have been tested on kernelci.org and do not see a problem.
An NXP/Freescale SABRESD i.MX6 Q board is requred.
My system for testing is to TFTP the Linux kernel over an ethernet connection. The
U-boot executes okay and the UART is working at that point. When the kernel loads
the console trace becomes garbled, in the sense that I get the some characters being
output to the console, in the style of the kernel starting up, but they are not correct.
I expect the kernel has started ok, but I am unable to read/write through the UART
console because of corruptions.
Console log with the output I am seeing with linux-next/v4.12-rc2
--- 8< ---
U-Boot 2009.08-00001-gf65536a (Jan 12 2015 - 15:47:19)
CPU: Freescale i.MX6 family TO1.2 at 792 MHz
Thermal sensor with ratio = 200
Temperature: 46 C, calibration data 0x5f15527d
mx6q pll1: 792MHz
mx6q pll2: 528MHz
mx6q pll3: 480MHz
mx6q pll8: 50MHz
ipg clock : 66000000Hz
ipg per clock : 66000000Hz
uart clock : 80000000Hz
cspi clock : 60000000Hz
ahb clock : 132000000Hz
axi clock : 264000000Hz
emi_slow clock: 132000000Hz
ddr clock : 528000000Hz
usdhc1 clock : 198000000Hz
usdhc2 clock : 198000000Hz
usdhc3 clock : 198000000Hz
usdhc4 clock : 198000000Hz
nfc clock : 24000000Hz
Board: i.MX6Q-SABRESD: unknown-board Board: 0x63012 [WDOG ]
Boot Device: SD
I2C: ready
DRAM: 1 GB
MMC: FSL_USDHC: 0,FSL_USDHC: 1,FSL_USDHC: 2,FSL_USDHC: 3
In: serial
Out: serial
Err: serial
Found PFUZE100! deviceid=10,revid=11
Net: got MAC address from IIM: 00:04:9f:02:e3:0a
FEC0 [PRIME]
Hit any key to stop autoboot: 0
PHY indentify @ 0x1 = 0x004dd074
FEC: Link is Up 796d
Using FEC0 device
TFTP from server 192.168.2.1; our IP address is 192.168.2.2
Filename 'uImage_dtb.imx6q.v4.12-rc2'.
Load address: 0x12000000
Loading: #################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
##########################################################
done
Bytes transferred = 5951108 (5ace84 hex)
## Booting kernel from Legacy Image at 12000000 ...
Image Name:
Image Type: ARM Linux Kernel Image (uncompressed)
Data Size: 5951044 Bytes = 5.7 MB
Load Address: 10800000
Entry Point: 10800000
Verifying Checksum ... OK
Loading Kernel Image ... OK
OK
Starting kernel ...
à ü ü pþ àüþü à àà àüþü àüü þü à ü à à àüŽ àü àü àà à à €à àüþü ààü üÿ
à àüŽü þü àüàð ààðàà à þ ~pà œà `þ ü àüŽà à à ü à þ üþ à ü à Žàà à
ààü üÿ à àüŽü þü àüàð €àà ààààðàà ðàüààüðààðààà þ üÿ üðü üŽ à üŽà à à üü
þ üÿ à àü ü pð üÀ üþü ü àà àüpÿ üààðàà ðàüàð à üàŽ ~8à Žàà Žààü€ààðààüp
Ž à à üà à à ààðàüðàà ðà üàpþàüðàüðààþà ðà üààà à ààðàà ðàüüàŽà €ü àà
Ž üà~ÿ üàà àà ü üŽà à ü þ àü~Žüàðààðààà à þà € ü ààð ü~ÿ àüŽà àüŽü þü à ðààà
Ž üà~ÿ üàŽà àü€ þ üà ðààü€ àüðààüüà€à üà €ü àà Ž àü àà ààã pŽ ààü ü
þü àü àüŽ à àüþü à àà àþ àü€ ü üð àà ààã pŽ à € àü àüŽ à àüþü à à Ž ü
--- 8< ---
Is this enough information for you?
It would be difficult to reproduce without the i.MX6Q (quad) board from Freescale/NXP
I think.
Regards,
Steve
^ permalink raw reply
* Re: [PATCH V1] serial: imx: revert setup DCEDTE early and ensure DCD and RI irqs to be off
From: Fabio Estevam @ 2017-05-24 11:52 UTC (permalink / raw)
To: Steve Twiss
Cc: Greg Kroah-Hartman, Jiri Slaby, LINUX-KERNEL, LINUX-SERIAL,
Lucas Stach, Uwe Kleine-Konig, Support Opensource
In-Reply-To: <6ED8E3B22081A4459DAC7699F3695FB7018CD8D75E@SW-EX-MBX02.diasemi.com>
Hi Steve,
On Wed, May 24, 2017 at 7:32 AM, Steve Twiss
<stwiss.opensource@diasemi.com> wrote:
> The USB to UART connection gets corrupted.
> If this patch is applied to the kernel, the i.MX6 Q (quad), and only this board as far as
> we know, starts to fail. This does *not* change the i.MX6DL and other sabre boards
> have been tested on kernelci.org and do not see a problem.
>
> An NXP/Freescale SABRESD i.MX6 Q board is requred.
>
> My system for testing is to TFTP the Linux kernel over an ethernet connection. The
> U-boot executes okay and the UART is working at that point. When the kernel loads
> the console trace becomes garbled, in the sense that I get the some characters being
> output to the console, in the style of the kernel starting up, but they are not correct.
>
> I expect the kernel has started ok, but I am unable to read/write through the UART
> console because of corruptions.
>
> Console log with the output I am seeing with linux-next/v4.12-rc2
> --- 8< ---
> U-Boot 2009.08-00001-gf65536a (Jan 12 2015 - 15:47:19)
Thanks for your detailed explanation.
I also have a mx6q sabresd board here, but the error did not happen
when I use a recent U-Boot version.
I would like to use the same U-Boot version here as well to help
debugging this problem.
Could you please let me know which 2009.08 branch you used? Is it any
one from git.freescale.com
http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/refs/heads ?
Thanks,
Fabio Estevam
^ permalink raw reply
* Re: [PATCH V1] serial: imx: revert setup DCEDTE early and ensure DCD and RI irqs to be off
From: Uwe Kleine-König @ 2017-05-24 11:57 UTC (permalink / raw)
To: Steve Twiss
Cc: Greg Kroah-Hartman, Jiri Slaby, LINUX-KERNEL, LINUX-SERIAL,
Lucas Stach, Support Opensource, kernel@pengutronix.de
In-Reply-To: <6ED8E3B22081A4459DAC7699F3695FB7018CD8D750@SW-EX-MBX02.diasemi.com>
Hello Steve,
On Wed, May 24, 2017 at 10:28:58AM +0000, Steve Twiss wrote:
> On 23 May 2017 17:09, Uwe Kleine-König wrote:
>
> > On Tue, May 23, 2017 at 03:01:26PM +0000, Steve Twiss wrote:
> > > On 23 May 2017 15:37, Uwe Kleine-König wrote:
> > > > Subject: Re: [PATCH V1] serial: imx: revert setup DCEDTE early and ensure DCD and RI irqs to be off
> > > > On Tue, May 23, 2017 at 02:28:11PM +0000, Steve Twiss wrote:
> > > > > On 23 May 2017 15:10, Uwe Kleine-König wrote:
> > > > > > On Tue, May 23, 2017 at 01:17:26PM +0100, Steve Twiss wrote:
> > > > > > >
> > > > > > > Revert the commit e61c38d85b7392e ("serial: imx: setup DCEDTE early and
> > > > > > > ensure DCD and RI irqs to be off")
> > > > > > > The patch submitted to setup DCEDTE early and ensure DCD and RI irqs to
> > > > > > > be off, causes a serial console display problem the i.MX6Q SABRESD board.
> > > > > > > The console becomes unreadable and unwritable.
> > > > > > >
> > > > > > > Tested-by: Steve Twiss <stwiss.opensource@diasemi.com>
> > > > > > > Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
> > > > > >
> > > > > > You're not the first to report this issue but you still have the chance
> > > > > > to be the first to test a suggested patch for it.
> > > > >
> > > > > I've just applied your patch against a clean linux-next/v4.12-rc2
> > > > > I added your patch ...
> > > > >
> > > > > > http://marc.info/?l=linux-serial&m=149434029912947&w=2
> > > > >
> > > [...]
> > > > > I've added that to my working directory, but I am still seeing the corrupted
> > > > > console output on the i.MX6 Q (quad) board.
> > > >
> > > > I don't have a failing board (I think). So here are a few questions
> > > > about yours:
> > > >
> > > > - how does the dts snippet for your failing device look like?
> > >
> > > I am using the standard DTS from the v4.12-rc2 kernel, no changes.
> > > I did an earlier test yesterday using the DTS from v4.11 to see if it was the new
> > > imx7 changes that have recently gone into the kernel but I still see the same
> > > effect.
> > >
> > > > - This is not the device the console runs on, right?
> > >
> > > I am connected through the USB to UART, U22 on the i.MX6Q board.
> > > Terminal set to 115200 baud, no parity, 8bit data.
> > >
> > > > - Can you initialize the device in the bootloader and check if it is working
> > there?
> > >
> > > I can get U-boot ok for all cases.
> > > Once I TFTP the kernel across, I am okay until I get to "Starting kernel ...",
> > > then, the UART is "working" in the sense that I get the some characters in the style
> > > of the kernel starting up, but they are all garbled.
> > >
> > > I expect the kernel has started ok, but I am unable to read/write through the UART
> > > console because of corruptions.
> > >
> > > Console log:
> > > --- 8< ---
> [...]
> > > Starting kernel ...
> > >
> > à\x1cü\x1cü\x1cpþ\x1càüþü\x1cà\x1càà\x1càüþü\x1c\x1c\x1c\x1càüü\x1cþü\x1cà\x1cü\x1cà\x1cà\x1càüŽ\x1c\x1càü\x1càü\x1c\x1c\x1c\x1c\x1càà\x1c
> [...]
> >
> > did you check with an oscilloscope if the baud rate is as expected (hmm,
> > but I wouldn't expect my patch to change the baud rate).
>
> I did try several different baudrates on the terminal connection, but I didn't find any
> that worked. I've only checked the obvious ones however. I have not tried to debug
> this problem any further than diagnosing what kernel commit caused the difference.
>
> We also swapped compilers to begin with, when the first investigation began.
> I noticed kernelci.org had some i.MX sabre boards, but used a different compiler
> and did not see any problem.
> Swapping the compiler made no difference and led us to find it only happened
> on the i.MX6Q not the i.MX6DL. The kernelci.org site does not test any i.MX6Q
> boards.
>
> > > > - Does it make a difference in Linux if the bootloader used the device before?
> > >
> > > If you mean, if U-boot uses the UART console before loading the kernel, then no.
> > > Is that what you mean?
> >
> > I didn't expect that it destroys the console UART so I expected that you
> > can make use of a 2nd UART in U-Boot somehow to already initialize the
> > port and check if that makes it magically work in Linux.
> >
> > Note to myself: So we're taking about the UART at 0x02020000, it is
> > operated in DCE mode.
>
> > > It makes no difference until the kernel is loaded, then the serial
> > > output gets corrupted.
> > >
> > > > - Can you dump the register space of the uart with v4.12-rc2 and
> > > > v4.12-rc2 + revert of e61c38d85b7392e?
> > >
> > > Difficult to do that I think. The console is unusable in both directions. I can't get any
> > > response from the console (through typing) once the kernel has started.
> >
> > ssh or telnet come to mind.
>
> Yup. We thought of that also, but finding the time at this side is the problem.
> I am looking at this Linux kernel i.MX6Q problem, but other customers are taking my
> priority at the moment, so this does not have a very high level (probably because
> we have found a "fix", at least to unblock our testing).
> Dialog do want to assist the Linux community however. So I will try to help where I can.
>
> > Can you try to just remove the line
> > writel(0, sport->port.membase + UFCR);
> > that was introduced in the last hunk by commit e61c38d85b7?
>
> Yep. I can do this.
> If I make this change, to the stock linux-next/v4.12-rc2 kernel, like this:
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 33509b4..68cfd3e 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -2194,9 +2194,7 @@ static int serial_imx_probe(struct platform_device *pdev)
> writel(IMX21_UCR3_RXDMUXSEL | UCR3_ADNIMP | UCR3_DSR,
> sport->port.membase + UCR3);
>
> - } else {
> - writel(0, sport->port.membase + UFCR);
> - }
> + }
>
> clk_disable_unprepare(sport->clk_ipg);
>
> This console works okay.
OK, thanks for testing. I understand now what is broken. I will check
where I can squeeze in a timeslot to create a patch.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH V1] serial: imx: revert setup DCEDTE early and ensure DCD and RI irqs to be off
From: Uwe Kleine-König @ 2017-05-24 12:02 UTC (permalink / raw)
To: Fabio Estevam
Cc: Steve Twiss, Greg Kroah-Hartman, Jiri Slaby, LINUX-KERNEL,
LINUX-SERIAL, Lucas Stach, Support Opensource
In-Reply-To: <CAOMZO5D=_L9Dd3gOgQssKBN5TouGqOx+zJnwr+-1HQrVT_yLRQ@mail.gmail.com>
Hey Fabio,
On Wed, May 24, 2017 at 08:52:50AM -0300, Fabio Estevam wrote:
> On Wed, May 24, 2017 at 7:32 AM, Steve Twiss
> <stwiss.opensource@diasemi.com> wrote:
>
> > The USB to UART connection gets corrupted.
> > If this patch is applied to the kernel, the i.MX6 Q (quad), and only this board as far as
> > we know, starts to fail. This does *not* change the i.MX6DL and other sabre boards
> > have been tested on kernelci.org and do not see a problem.
> >
> > An NXP/Freescale SABRESD i.MX6 Q board is requred.
> >
> > My system for testing is to TFTP the Linux kernel over an ethernet connection. The
> > U-boot executes okay and the UART is working at that point. When the kernel loads
> > the console trace becomes garbled, in the sense that I get the some characters being
> > output to the console, in the style of the kernel starting up, but they are not correct.
> >
> > I expect the kernel has started ok, but I am unable to read/write through the UART
> > console because of corruptions.
> >
> > Console log with the output I am seeing with linux-next/v4.12-rc2
> > --- 8< ---
> > U-Boot 2009.08-00001-gf65536a (Jan 12 2015 - 15:47:19)
>
> Thanks for your detailed explanation.
>
> I also have a mx6q sabresd board here, but the error did not happen
> when I use a recent U-Boot version.
>
> I would like to use the same U-Boot version here as well to help
> debugging this problem.
>
> Could you please let me know which 2009.08 branch you used? Is it any
> one from git.freescale.com
> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/refs/heads ?
The problem is that RFDIV, RXTL and TXTL are overwritten in the UFCR
register.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* RE: [PATCH V1] serial: imx: revert setup DCEDTE early and ensure DCD and RI irqs to be off
From: Steve Twiss @ 2017-05-24 12:49 UTC (permalink / raw)
To: Fabio Estevam
Cc: Greg Kroah-Hartman, Jiri Slaby, LINUX-KERNEL, LINUX-SERIAL,
Lucas Stach, Uwe Kleine-Konig, Support Opensource
In-Reply-To: <CAOMZO5D=_L9Dd3gOgQssKBN5TouGqOx+zJnwr+-1HQrVT_yLRQ@mail.gmail.com>
Hi Fabio,
On 24 May 2017 12:53 Fabio Estevam wrote:
> Subject: Re: [PATCH V1] serial: imx: revert setup DCEDTE early and ensure DCD and RI irqs to be off
> On Wed, May 24, 2017 at 7:32 AM, Steve Twiss wrote:
>
> > The USB to UART connection gets corrupted.
> > If this patch is applied to the kernel, the i.MX6 Q (quad), and only this board as far as
> > we know, starts to fail. This does *not* change the i.MX6DL and other sabre boards
> > have been tested on kernelci.org and do not see a problem.
> >
> > An NXP/Freescale SABRESD i.MX6 Q board is requred.
> >
> > My system for testing is to TFTP the Linux kernel over an ethernet connection. The
> > U-boot executes okay and the UART is working at that point. When the kernel loads
> > the console trace becomes garbled, in the sense that I get the some characters being
> > output to the console, in the style of the kernel starting up, but they are not correct.
> >
> > I expect the kernel has started ok, but I am unable to read/write through the UART
> > console because of corruptions.
> >
> > Console log with the output I am seeing with linux-next/v4.12-rc2
> > --- 8< ---
> > U-Boot 2009.08-00001-gf65536a (Jan 12 2015 - 15:47:19)
>
> Thanks for your detailed explanation.
>
> I also have a mx6q sabresd board here, but the error did not happen
> when I use a recent U-Boot version.
>
> I would like to use the same U-Boot version here as well to help
> debugging this problem.
For the previous tests I was using a slightly modified U-Boot from an old Freescale
Android release JB43_110, but I've just compiled up the standard u-boot from
the Freescale git repository, and I get the same results.
> Could you please let me know which 2009.08 branch you used? Is it any
> one from git.freescale.com
> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/refs/heads ?
I used this tag: jb4.3_1.1.1-ga
http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?id=jb4.3_1.1.1-ga
This repo is slightly modified for a project I am working on, but it only queries
the I2C for devices before printing out some messages, so minor changes I
think. Here's the output from the console:
--- 8< ---
U-Boot 2009.08 (May 24 2017 - 13:15:47)
CPU: Freescale i.MX6 family TO1.2 at 792 MHz
Thermal sensor with ratio = 200
Temperature: 38 C, calibration data 0x5f15527d
mx6q pll1: 792MHz
mx6q pll2: 528MHz
mx6q pll3: 480MHz
mx6q pll8: 50MHz
ipg clock : 66000000Hz
ipg per clock : 66000000Hz
uart clock : 80000000Hz
cspi clock : 60000000Hz
ahb clock : 132000000Hz
axi clock : 264000000Hz
emi_slow clock: 132000000Hz
ddr clock : 528000000Hz
usdhc1 clock : 198000000Hz
usdhc2 clock : 198000000Hz
usdhc3 clock : 198000000Hz
usdhc4 clock : 198000000Hz
nfc clock : 24000000Hz
Board: i.MX6Q-SABRESD: unknown-board Board: 0x63012 [POR ]
Boot Device: SD
I2C: ready
DRAM: 1 GB
MMC: FSL_USDHC: 0,FSL_USDHC: 1,FSL_USDHC: 2,FSL_USDHC: 3
In: serial
Out: serial
Err: serial
Found PFUZE100! deviceid=10,revid=11
Net: got MAC address from IIM: 00:04:9f:02:e3:0a
FEC0 [PRIME]
Hit any key to stop autoboot: 0
PHY indentify @ 0x1 = 0x004dd074
FEC: Link is Up 796d
Using FEC0 device
TFTP from server 192.168.2.1; our IP address is 192.168.2.2
Filename 'uImage_dtb.imx6q.v4.12-rc2'.
Load address: 0x12000000
Loading: #################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
#################################################################
##########################################################
done
Bytes transferred = 5951076 (5ace64 hex)
## Booting kernel from Legacy Image at 12000000 ...
Image Name:
Image Type: ARM Linux Kernel Image (uncompressed)
Data Size: 5951012 Bytes = 5.7 MB
Load Address: 10800000
Entry Point: 10800000
Verifying Checksum ... OK
Loading Kernel Image ... OK
OK
--- 8< ---
Regards,
Stephen
^ permalink raw reply
* Re: [PATCH 1/1] xilinx ps uart: Adding a kernel parameter for the number of xilinx ps uarts
From: Michal Simek @ 2017-05-24 13:06 UTC (permalink / raw)
To: Alan Cox, Michal Simek
Cc: gregkh, linux-kernel, Rob Herring, Sam Povilus, linux-arm-kernel,
linux-serial, jslaby, soren.brinkmann
In-Reply-To: <20170523210710.4e84dd2d@alans-desktop>
On 23.5.2017 22:07, Alan Cox wrote:
>> yep hardcoded max 4 where in probe first free space is found and used
>> (range 0-3) but still max3100s statically allocated.
>> Shouldn't be this also dynamically allocated?
>
> The code to do the dynamic allocation would be larger than the array of
> pointers for the sane worst case.
>
>> I am not quite sure how exactly you want to do this via DT.
>
> Count the number of DT entries for this kind of port and allocate that
> many ?
:-) I think there is no doubt how to calculate that number for
static/fixed system. But where do you want to put it? And who should
read it?
A lot of drivers are calling uart_register_driver in module_init.
Others are calling it in probe(pl011 for example).
In module init you probably don't have a pointer to DT to read this and
this should be in generic location not in node itself.
If all drivers should call it from probe then we should change that -
then reading property is easy and only location should be cleared.
That's why I am curious how exactly you would do it because I haven't
seen this before.
>
>>
>> Also what do you think is a safe maximum number? This is fpga - hundreds
>> of pins which can do just uart.
>>
>>> There are lots of options better than breaking the "one kernel many
>>> platforms" model.
>>
>> Another options is also module parameter and dynamically allocated array
>> in cdns_uart_init.
>
> For a given platform the number is constant and they need to be
> described, so it seems to make no sense to put it anywhere other than the
> DT for that platform.
Also I don't think this is really constant in all cases. Especially in
connection to fpga where you can put others IPs to PL. You never know
what it is present in partial region and how many of these are there.
It means having truly dynamic behavior would be welcome.
Can you call uart_register_driver() from probe for every instance? It
means nr is 1 all the time.
>
> Why should users have to pass magic config options not use DT as
> intended ?
I am not saying that config option is perfect solution. It is at least
aligned with 5 others serial drivers in the tree.
Thanks,
Michal
^ permalink raw reply
* Re: [PATCH 1/1] xilinx ps uart: Adding a kernel parameter for the number of xilinx ps uarts
From: Alan Cox @ 2017-05-24 13:31 UTC (permalink / raw)
To: Michal Simek
Cc: gregkh, linux-kernel, Rob Herring, Sam Povilus, linux-arm-kernel,
linux-serial, jslaby, soren.brinkmann
In-Reply-To: <924433a1-7e27-6eb6-71ca-ed7baceb427f@xilinx.com>
> I am not saying that config option is perfect solution. It is at least
> aligned with 5 others serial drivers in the tree.
And the fact people keep doing hacks jutifies continuing to make a mess.
Especialy as in this case it's entirely theoretical. Nobody has produced
actual hardware that hits the limit. Nobody has filed a bug, nobody is
impacted.
Creating extra CONFIG_ entries for junk like this is ridiculous, most of
the others at least have the excuse of being old code.
Generally it is better to call uart_register_driver from the probe method
because that way you don't waste time and memory registering drivers for
stuff that isn't even present however if you have no idea how many
devices there might be then you still really need to pass a suitable limit
and handle it internally dynamically allocating as needed.
If someone was hitting this in the real world and you posted a patch that
just changed the constant to 8 or 16 or whatever was needed I wouldn't
care too much, but adding CONFIG_ entries just makes stuff harder and
harder to config and more and more impossible to keep generic.
I keep hearing that the ARM folks are trying to get one unified kernel.
CONFIG_ options is not how to do that.
Alan
^ permalink raw reply
* Re: [PATCH V1] serial: imx: revert setup DCEDTE early and ensure DCD and RI irqs to be off
From: Fabio Estevam @ 2017-05-24 13:52 UTC (permalink / raw)
To: Steve Twiss
Cc: Greg Kroah-Hartman, Jiri Slaby, LINUX-KERNEL, LINUX-SERIAL,
Lucas Stach, Uwe Kleine-Konig, Support Opensource
In-Reply-To: <6ED8E3B22081A4459DAC7699F3695FB7018CD8D7CD@SW-EX-MBX02.diasemi.com>
Hi Steve,
On Wed, May 24, 2017 at 9:49 AM, Steve Twiss
<stwiss.opensource@diasemi.com> wrote:
> I used this tag: jb4.3_1.1.1-ga
> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?id=jb4.3_1.1.1-ga
>
> This repo is slightly modified for a project I am working on, but it only queries
> the I2C for devices before printing out some messages, so minor changes I
> think. Here's the output from the console:
I am not able to reproduce it using this same U-Boot version on my
mx6sabresd board:
https://pastebin.com/ppnrnidr
Had to select dtb append in the kernel menu config.
^ permalink raw reply
* Aw: Re: [PATCH v6 net-next 17/17] net: qualcomm: add QCA7000 UART driver
From: Lino Sanfilippo @ 2017-05-24 14:19 UTC (permalink / raw)
To: Stefan Wahren
Cc: Rob Herring, Mark Rutland, David S. Miller, linux-serial,
Jiri Slaby, Greg Kroah-Hartman, netdev, linux-kernel,
Jakub Kicinski, devicetree
In-Reply-To: <50e5a442-777f-1516-4e94-16db7fa28f8b@i2se.com>
Hi,
> Gesendet: Mittwoch, 24. Mai 2017 um 11:06 Uhr
> Von: "Stefan Wahren" <stefan.wahren@i2se.com>
> An: "Lino Sanfilippo" <LinoSanfilippo@gmx.de>, "Rob Herring" <robh+dt@kernel.org>, "Mark Rutland" <mark.rutland@arm.com>, "David S. Miller" <davem@davemloft.net>
> Cc: linux-serial@vger.kernel.org, "Jiri Slaby" <jslaby@suse.com>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, "Jakub Kicinski" <kubakici@wp.pl>, devicetree@vger.kernel.org
> Betreff: Re: [PATCH v6 net-next 17/17] net: qualcomm: add QCA7000 UART driver
>
> Am 23.05.2017 um 23:01 schrieb Lino Sanfilippo:
> > On 23.05.2017 21:38, Stefan Wahren wrote:
> >>> Lino Sanfilippo <LinoSanfilippo@gmx.de> hat am 23. Mai 2017 um 20:16 geschrieben:
> >>>
> >>> I suggest to avoid this possible race by first unregistering the netdevice and then
> >>> calling cancel_work_sync().
> >> What makes you sure that's safe to unregister the netdev while the tx work queue is possibly active?
> > unregister_netdevice() calls netdev_close() if the interface is still up. netdev_close() calls flush_work()
> > so the unregistration is delayed until the tx work function is finished. Furthermore both close() and
> > tx work are synchronized by means of the qca->lock which also guarantees that unregister_netdevice() wont
> > be finished until the tx work is done.
> >
>
> Thanks for the explanation. I suspect there could be the same race
> between serdev_device_close() and the tx work queue.
>
> So i would propose a variant of your original suggestion:
>
> unregister_netdev(qca->net_dev);
>
> /* Flush any pending characters in the driver. */
> serdev_device_close(serdev);
> cancel_work_sync(&qca->tx_work);
>
> Since we have the same pattern in the error path of the probe function,
> the same applies there.
Agreed, it is much cleaner to have the same cleanup pattern in remove() as
we have in (error case of) probe().
Regards,
Lino
^ permalink raw reply
* Re: [PATCH RESEND v7 net-next 17/17] net: qualcomm: add QCA7000 UART driver
From: Lino Sanfilippo @ 2017-05-24 14:21 UTC (permalink / raw)
Cc: Rob Herring, Mark Rutland, David S. Miller, Greg Kroah-Hartman,
Jiri Slaby, Jakub Kicinski, devicetree, netdev, linux-serial,
linux-kernel, Stefan Wahren
In-Reply-To: <1495618330-1314-1-git-send-email-stefan.wahren@i2se.com>
Hi,
> This patch adds the Ethernet over UART driver for the
> Qualcomm QCA7000 HomePlug GreenPHY.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Regards,
Lino
^ permalink raw reply
* Re: [PATCH RESEND v7 net-next 17/17] net: qualcomm: add QCA7000 UART driver
From: Stefan Wahren @ 2017-05-24 14:34 UTC (permalink / raw)
To: Lino Sanfilippo
Cc: Rob Herring, Mark Rutland, David S. Miller, Greg Kroah-Hartman,
Jiri Slaby, Jakub Kicinski, devicetree-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <trinity-190c217d-ab18-4d21-b7a5-fd1d6a3ea051-1495635717353@3capp-gmx-bs75>
Am 24.05.2017 um 16:21 schrieb Lino Sanfilippo:
> Hi,
>
>
>> This patch adds the Ethernet over UART driver for the
>> Qualcomm QCA7000 HomePlug GreenPHY.
>>
>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>> ---
> Reviewed-by: Lino Sanfilippo <LinoSanfilippo-Mmb7MZpHnFY@public.gmane.org>
>
Thanks
Stefan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
From: Babu Moger @ 2017-05-24 14:45 UTC (permalink / raw)
To: Arnd Bergmann, Geert Uytterhoeven
Cc: David S. Miller, Peter Zijlstra, Ingo Molnar, sparclinux,
linux-kernel@vger.kernel.org, Linux-Arch,
devicetree@vger.kernel.org, linux-serial@vger.kernel.org
In-Reply-To: <CAK8P3a1XZGQk3b3q+JFJ+mLgw_upUdAtTM1VBjHZq5f2m-YdUg@mail.gmail.com>
Arnd,
On 5/24/2017 5:18 AM, Arnd Bergmann wrote:
> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
>>> Found this problem while enabling queued rwlock on SPARC.
>>> The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
>>> specific byte in qrwlock structure. Without this parameter,
>>> we clear the wrong byte. Here is the code.
>>>
>>> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>>> {
>>> return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>>> }
>>>
>>> Define CPU_BIG_ENDIAN for SPARC to fix it.
>>> --- a/arch/sparc/Kconfig
>>> +++ b/arch/sparc/Kconfig
>>> @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
>>> config ARCH_PROC_KCORE_TEXT
>>> def_bool y
>>>
>>> +config CPU_BIG_ENDIAN
>>> + bool
>>> + default y if SPARC
>> Nice catch!
>>
>> Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
>> architectures that may support both. And it was checked in platform code
>> and drivers only.
>> Hence the symbol is lacking from most architectures. Heck, even
>> architectures that support both may default to one endiannes, and declare
>> only the symbol for the other endianness:
>>
>> --- arch/alpha ---
>> --- arch/arc ---
>> arch/arc/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/arm ---
>> arch/arm/mm/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/arm64 ---
>> arch/arm64/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/blackfin ---
>> --- arch/c6x ---
>> arch/c6x/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/cris ---
>> --- arch/frv ---
>> --- arch/h8300 ---
>> --- arch/hexagon ---
>> --- arch/ia64 ---
>> --- arch/Kconfig ---
>> --- arch/m32r ---
>> arch/m32r/Kconfig:config CPU_LITTLE_ENDIAN
>> --- arch/m68k ---
>> --- arch/metag ---
>> --- arch/microblaze ---
>> --- arch/mips ---
>> arch/mips/Kconfig:config CPU_BIG_ENDIAN
>> arch/mips/Kconfig:config CPU_LITTLE_ENDIAN
>> --- arch/mn10300 ---
>> --- arch/nios2 ---
>> --- arch/openrisc ---
>> --- arch/parisc ---
>> --- arch/powerpc ---
>> arch/powerpc/platforms/Kconfig.cputype:config CPU_BIG_ENDIAN
>> arch/powerpc/platforms/Kconfig.cputype:config CPU_LITTLE_ENDIAN
>> --- arch/s390 ---
>> arch/s390/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/score ---
>> --- arch/sh ---
>> arch/sh/Kconfig.cpu:config CPU_LITTLE_ENDIAN
>> arch/sh/Kconfig.cpu:config CPU_BIG_ENDIAN
>> --- arch/sparc ---
>> --- arch/tile ---
>> --- arch/um ---
>> --- arch/unicore32 ---
>> --- arch/x86 ---
>> --- arch/xtensa ---
>>
>> However, there are already a few users in generic code, which are thus
>> broken on many platforms:
>>
>> drivers/of/base.c
>> drivers/of/fdt.c
>> drivers/tty/serial/earlycon.c
>> drivers/tty/serial/serial_core.c
>>
>> include/asm-generic/qrwlock.h is also generic, but depends on the
>> architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
>> (x86, and now sparc).
>>
>> I guess the time is ripe for adding (both) symbols to all architectures?
> Good idea. I think we can do most of this by adding a few lines to
> arch/Kconfig:
>
> config CPU_BIG_ENDIAN
> bool
>
> config CPU_LITTLE_ENDIAN
> def_bool !CPU_BIG_ENDIAN
I noticed that even x86 does not define CPU_LITTLE_ENDIAN. Strange.
With this code all the architecture will default to
CONFIG_CPU_LITTLE_ENDIAN.
I can make it as a separate patch. But I can only test SPARC and little
bit of x86.
Is that ok?
>
> This way, we only need to add 'select CPU_BIG_ENDIAN' to the
> architectures that are always big-endian, and we don't need to
> change anything for the ones that have a single 'CPU_BIG_ENDIAN'
> option.
>
> The three architectures that have a 'choice' statement (mips, ppc and
> sh) will have to convert, and m32r will have to replace the
> option with the opposite one, which could break 'make oldconfig',
> but nobody really cares about m32r any more.
>
> Arnd
^ permalink raw reply
* Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
From: Arnd Bergmann @ 2017-05-24 15:09 UTC (permalink / raw)
To: Babu Moger
Cc: Geert Uytterhoeven, David S. Miller, Peter Zijlstra, Ingo Molnar,
sparclinux, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux-Arch, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <8ec12d9b-30da-6088-e340-93cc92245ed4-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
On Wed, May 24, 2017 at 4:45 PM, Babu Moger <babu.moger-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> On 5/24/2017 5:18 AM, Arnd Bergmann wrote:
>> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
>> <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
>>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>>> wrote:
>>> include/asm-generic/qrwlock.h is also generic, but depends on the
>>> architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
>>> (x86, and now sparc).
>>>
>>> I guess the time is ripe for adding (both) symbols to all architectures?
>>
>> Good idea. I think we can do most of this by adding a few lines to
>> arch/Kconfig:
>>
>> config CPU_BIG_ENDIAN
>> bool
>>
>> config CPU_LITTLE_ENDIAN
>> def_bool !CPU_BIG_ENDIAN
>
> I noticed that even x86 does not define CPU_LITTLE_ENDIAN. Strange.
There is no architecture-independent code that tests for
CONFIG_CPU_LITTLE_ENDIAN, unlike CONFIG_CPU_BIG_ENDIAN,
so that's not very suprising.
> With this code all the architecture will default to
> CONFIG_CPU_LITTLE_ENDIAN.
What I meant is that we have to 'select CPU_BIG_ENDIAN' on all architectures
that actually are big-endian:
These are all configurable:
$ git grep -l linux/byteorder/big_endian.h | xargs grep -l
linux/byteorder/little_endian.h
arch/arc/include/uapi/asm/byteorder.h
arch/arm/include/uapi/asm/byteorder.h
arch/arm64/include/uapi/asm/byteorder.h
arch/c6x/include/uapi/asm/byteorder.h
arch/m32r/include/uapi/asm/byteorder.h
arch/microblaze/include/uapi/asm/byteorder.h
arch/mips/include/uapi/asm/byteorder.h
arch/powerpc/include/uapi/asm/byteorder.h
arch/sh/include/uapi/asm/byteorder.h
arch/tile/include/uapi/asm/byteorder.h
These are always big-endian:
$ git grep -l linux/byteorder/big_endian.h | xargs grep -L
linux/byteorder/little_endian.h
arch/avr32/include/uapi/asm/byteorder.h
arch/frv/include/uapi/asm/byteorder.h
arch/m68k/include/uapi/asm/byteorder.h
arch/openrisc/include/uapi/asm/byteorder.h
arch/parisc/include/uapi/asm/byteorder.h
arch/s390/include/uapi/asm/byteorder.h
arch/sparc/include/uapi/asm/byteorder.h
And these are always little-endian:
arch/alpha/include/uapi/asm/byteorder.h
arch/blackfin/include/uapi/asm/byteorder.h
arch/cris/include/uapi/asm/byteorder.h
arch/hexagon/include/uapi/asm/byteorder.h
arch/ia64/include/uapi/asm/byteorder.h
arch/metag/include/uapi/asm/byteorder.h
arch/mn10300/include/uapi/asm/byteorder.h
arch/score/include/uapi/asm/byteorder.h
arch/unicore32/include/uapi/asm/byteorder.h
arch/x86/include/uapi/asm/byteorder.h
So if we 'select CPU_BIG_ENDIAN' from avr32, frv, m68k, openrisc, parisc,
s390 and sparc, this covers all the fixed-endian architectures, and the
other ones are those that already have either CPU_BIG_ENDIAN
as a 'bool' option, or both as a 'choice'.
> I can make it as a separate patch. But I can only test SPARC and little bit
> of x86. Is that ok?
I think that's ok.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH V1] serial: imx: revert setup DCEDTE early and ensure DCD and RI irqs to be off
From: Steve Twiss @ 2017-05-24 16:08 UTC (permalink / raw)
To: Fabio Estevam
Cc: Greg Kroah-Hartman, Jiri Slaby, LINUX-KERNEL, LINUX-SERIAL,
Lucas Stach, Uwe Kleine-Konig, Support Opensource
In-Reply-To: <CAOMZO5CYaZZES6VSUE2JpCvHSH+fvr13ECZ=oKKoKRpmQUXDaA@mail.gmail.com>
Hi Fabio,
On 24 May 2017 14:53 Fabio Estevam wrote,
> Subject: Re: [PATCH V1] serial: imx: revert setup DCEDTE early and ensure DCD and RI irqs to be off
> On Wed, May 24, 2017 at 9:49 AM, Steve Twiss wrote:
>
> > I used this tag: jb4.3_1.1.1-ga
> > http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?id=jb4.3_1.1.1-ga
> >
> > This repo is slightly modified for a project I am working on, but it only queries
> > the I2C for devices before printing out some messages, so minor changes I
> > think. Here's the output from the console:
>
> I am not able to reproduce it using this same U-Boot version on my
> mx6sabresd board:
>
> https://pastebin.com/ppnrnidr
That's very interesting.
Once I apply the test change from Uwe, I can get a console trace.
> Uwe Kleine-König wrote:
> > Can you try to just remove the line
> > writel(0, sport->port.membase + UFCR);
The differences in the kernel console logs are ...
My board : Your board differences
Board: i.MX6Q-SABRESD: unknown-board Board: 0x63012 [POR ] : Board: i.MX6Q-SABRESD: unknown-board Board: 0x63011 [POR ]
Found PFUZE100! deviceid=10,revid=11 : Found PFUZE100! deviceid=10,revid=10
... and others, but.. I do not have "console=ttymxc0,115200" in my kernel
command line.
Yes. I can verify, using the raw linux-next/v4.12-rc2 I get the console error.
When I add "console=ttymxc0,115200" to my U-boot environment and the
kernel command-line, the kernel console starts working again.
That would indicate the test on the i.MX6DL might be misleading, since my
colleague ran that using their bootloader set-up. So, my assertion this was
only visible on the i.MX6Q could be wrong.
There's no way to to check this until tomorrow now.
I'll check check on the i.MX6DL using my bootloader environment when I
get access to an i.MX6DL board.
Regards,
Stephen
^ permalink raw reply
* Re: [PATCH 1/1] xilinx ps uart: Adding a kernel parameter for the number of xilinx ps uarts
From: Michal Simek @ 2017-05-24 16:09 UTC (permalink / raw)
To: Alan Cox, Michal Simek
Cc: Rob Herring, Sam Povilus, gregkh, jslaby, soren.brinkmann,
linux-serial, linux-arm-kernel, linux-kernel
In-Reply-To: <20170524143108.682a548d@alans-desktop>
On 24.5.2017 15:31, Alan Cox wrote:
>> I am not saying that config option is perfect solution. It is at least
>> aligned with 5 others serial drivers in the tree.
>
> And the fact people keep doing hacks jutifies continuing to make a mess.
> Especialy as in this case it's entirely theoretical. Nobody has produced
> actual hardware that hits the limit. Nobody has filed a bug, nobody is
> impacted.
This is the reason why we are talking about it how to do it right.
With this ps uart it is not that easy because this is cadence RTL which
is not in public IP database but the same think is with uartlite.
Limit there is 16. If you really want that I can create that HW design
which will require more than 16 uartlites in one design.
> Creating extra CONFIG_ entries for junk like this is ridiculous, most of
> the others at least have the excuse of being old code.
No doubt about it. I am just trying to find out what's the way you are
suggesting.
> Generally it is better to call uart_register_driver from the probe method
> because that way you don't waste time and memory registering drivers for
> stuff that isn't even present however if you have no idea how many
> devices there might be then you still really need to pass a suitable limit
> and handle it internally dynamically allocating as needed.
Ok. Is there any problem if uart_register_driver is called for every
instance separately with nr=1? This driver has major 0, minor 0. Based
on comment major 0 is for dynamic node allocation. Not sure about minor
but it is easy to figured out if this should be 0 or 1, 2, 3, etc.
> If someone was hitting this in the real world and you posted a patch that
> just changed the constant to 8 or 16 or whatever was needed I wouldn't
> care too much, but adding CONFIG_ entries just makes stuff harder and
> harder to config and more and more impossible to keep generic.
>
> I keep hearing that the ARM folks are trying to get one unified kernel.
> CONFIG_ options is not how to do that.
I have really not a problem with all of this. Just trying to understand
how to do it properly and cleanup the second driver which we use on
fpga. Last driver used by Xilinx is uart16550 where that old config
macro already exists.
Because at least now there is an issue in driver if you use serial
aliases (serial2 and up) which needs to be fixed.
Thanks,
Michal
^ permalink raw reply
* Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
From: Babu Moger @ 2017-05-24 17:03 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, David S. Miller, Peter Zijlstra, Ingo Molnar,
sparclinux, linux-kernel@vger.kernel.org, Linux-Arch,
devicetree@vger.kernel.org, linux-serial@vger.kernel.org
In-Reply-To: <CAK8P3a2gk07SW2nGZdNXSzRwx5+Ap5hDf4wDksunLtYkpC5WOg@mail.gmail.com>
On 5/24/2017 10:09 AM, Arnd Bergmann wrote:
> On Wed, May 24, 2017 at 4:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
>> On 5/24/2017 5:18 AM, Arnd Bergmann wrote:
>>> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com>
>>>> wrote:
>>>> include/asm-generic/qrwlock.h is also generic, but depends on the
>>>> architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
>>>> (x86, and now sparc).
>>>>
>>>> I guess the time is ripe for adding (both) symbols to all architectures?
>>> Good idea. I think we can do most of this by adding a few lines to
>>> arch/Kconfig:
>>>
>>> config CPU_BIG_ENDIAN
>>> bool
>>>
>>> config CPU_LITTLE_ENDIAN
>>> def_bool !CPU_BIG_ENDIAN
>> I noticed that even x86 does not define CPU_LITTLE_ENDIAN. Strange.
> There is no architecture-independent code that tests for
> CONFIG_CPU_LITTLE_ENDIAN, unlike CONFIG_CPU_BIG_ENDIAN,
> so that's not very suprising.
Ok. Thanks
>> With this code all the architecture will default to
>> CONFIG_CPU_LITTLE_ENDIAN.
> What I meant is that we have to 'select CPU_BIG_ENDIAN' on all architectures
> that actually are big-endian:
Ok. Sure.
>
> These are all configurable:
> $ git grep -l linux/byteorder/big_endian.h | xargs grep -l
> linux/byteorder/little_endian.h
> arch/arc/include/uapi/asm/byteorder.h
> arch/arm/include/uapi/asm/byteorder.h
> arch/arm64/include/uapi/asm/byteorder.h
> arch/c6x/include/uapi/asm/byteorder.h
> arch/m32r/include/uapi/asm/byteorder.h
> arch/microblaze/include/uapi/asm/byteorder.h
> arch/mips/include/uapi/asm/byteorder.h
> arch/powerpc/include/uapi/asm/byteorder.h
> arch/sh/include/uapi/asm/byteorder.h
> arch/tile/include/uapi/asm/byteorder.h
>
> These are always big-endian:
> $ git grep -l linux/byteorder/big_endian.h | xargs grep -L
> linux/byteorder/little_endian.h
> arch/avr32/include/uapi/asm/byteorder.h
> arch/frv/include/uapi/asm/byteorder.h
> arch/m68k/include/uapi/asm/byteorder.h
> arch/openrisc/include/uapi/asm/byteorder.h
> arch/parisc/include/uapi/asm/byteorder.h
> arch/s390/include/uapi/asm/byteorder.h
> arch/sparc/include/uapi/asm/byteorder.h
>
> And these are always little-endian:
> arch/alpha/include/uapi/asm/byteorder.h
> arch/blackfin/include/uapi/asm/byteorder.h
> arch/cris/include/uapi/asm/byteorder.h
> arch/hexagon/include/uapi/asm/byteorder.h
> arch/ia64/include/uapi/asm/byteorder.h
> arch/metag/include/uapi/asm/byteorder.h
> arch/mn10300/include/uapi/asm/byteorder.h
> arch/score/include/uapi/asm/byteorder.h
> arch/unicore32/include/uapi/asm/byteorder.h
> arch/x86/include/uapi/asm/byteorder.h
>
> So if we 'select CPU_BIG_ENDIAN' from avr32, frv, m68k, openrisc, parisc,
> s390 and sparc, this covers all the fixed-endian architectures, and the
> other ones are those that already have either CPU_BIG_ENDIAN
> as a 'bool' option, or both as a 'choice'.
Ok. Great details. I think I have all the details required for the
first version. Will post it soon. Thanks
>> I can make it as a separate patch. But I can only test SPARC and little bit
>> of x86. Is that ok?
> I think that's ok.
>
> Arnd
^ permalink raw reply
* Re: [PATCH v6 net-next 01/17] net: qualcomm: remove unnecessary includes
From: David Miller @ 2017-05-24 19:41 UTC (permalink / raw)
To: stefan.wahren-eS4NqCHxEME
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jslaby-IBi9RG/b67k,
LinoSanfilippo-Mmb7MZpHnFY, kubakici-5tc4TXWwyLM,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1495545173-22150-2-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
From: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
Date: Tue, 23 May 2017 15:12:37 +0200
> Most of the includes in qca_7k.c are unnecessary so we better remove them.
>
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
> drivers/net/ethernet/qualcomm/qca_7k.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/qualcomm/qca_7k.c b/drivers/net/ethernet/qualcomm/qca_7k.c
> index f0066fb..557d53c 100644
> --- a/drivers/net/ethernet/qualcomm/qca_7k.c
> +++ b/drivers/net/ethernet/qualcomm/qca_7k.c
> @@ -23,11 +23,7 @@
> * kernel-based SPI device.
> */
>
> -#include <linux/init.h>
> -#include <linux/module.h>
> -#include <linux/moduleparam.h>
> #include <linux/spi/spi.h>
> -#include <linux/version.h>
>
> #include "qca_7k.h"
>
> --
> 2.1.4
>
Changes like this drive me crazy.
The only reason you can remove those headers is because you are obtaining
things indirectly via qca_7k.h
And if that is indeed the case, you are also getting qca_spi.h which
in turn includes linux/spi/spi.h
So you could have removed that as well.
But seriously, it is so much harder to understand a driver and what
interfaces it needs via header files when you hide _all_ of it behind
these local private header files which just include _everything_
and then _every_ foo.c file in your driver gets _all_ of those kernel
headers whether they need it or not.
So if just one foo.c file needs 20 extra kernel headers than the rest
of the files in the driver, every foo.c file eats that cost of
including them.
I really don't like when drivers move in this direction for that
reason. And at best, as described at the beginning of my response,
this change is incomplete.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 net-next 01/17] net: qualcomm: remove unnecessary includes
From: Stefan Wahren @ 2017-05-24 20:05 UTC (permalink / raw)
To: David Miller
Cc: linux-serial, jslaby, gregkh, netdev, robh+dt, linux-kernel,
kubakici, mark.rutland, LinoSanfilippo, devicetree
In-Reply-To: <20170524.154111.404338711401048909.davem@davemloft.net>
> David Miller <davem@davemloft.net> hat am 24. Mai 2017 um 21:41 geschrieben:
>
>
> From: Stefan Wahren <stefan.wahren@i2se.com>
> Date: Tue, 23 May 2017 15:12:37 +0200
>
> > Most of the includes in qca_7k.c are unnecessary so we better remove them.
> >
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > ---
> > drivers/net/ethernet/qualcomm/qca_7k.c | 4 ----
> > 1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/qualcomm/qca_7k.c b/drivers/net/ethernet/qualcomm/qca_7k.c
> > index f0066fb..557d53c 100644
> > --- a/drivers/net/ethernet/qualcomm/qca_7k.c
> > +++ b/drivers/net/ethernet/qualcomm/qca_7k.c
> > @@ -23,11 +23,7 @@
> > * kernel-based SPI device.
> > */
> >
> > -#include <linux/init.h>
> > -#include <linux/module.h>
> > -#include <linux/moduleparam.h>
> > #include <linux/spi/spi.h>
> > -#include <linux/version.h>
> >
> > #include "qca_7k.h"
> >
> > --
> > 2.1.4
> >
>
> Changes like this drive me crazy.
>
> The only reason you can remove those headers is because you are obtaining
> things indirectly via qca_7k.h
>
> And if that is indeed the case, you are also getting qca_spi.h which
> in turn includes linux/spi/spi.h
>
> So you could have removed that as well.
>
> But seriously, it is so much harder to understand a driver and what
> interfaces it needs via header files when you hide _all_ of it behind
> these local private header files which just include _everything_
> and then _every_ foo.c file in your driver gets _all_ of those kernel
> headers whether they need it or not.
>
> So if just one foo.c file needs 20 extra kernel headers than the rest
> of the files in the driver, every foo.c file eats that cost of
> including them.
>
> I really don't like when drivers move in this direction for that
> reason. And at best, as described at the beginning of my response,
> this change is incomplete.
>
The intension of this change wasn't to hide the includes into qca_7k.h
AFAIK these ones above aren't necessary (no init, no kernel module, no kernel parameter, no kernel version) for this C file. So i will double check it.
^ permalink raw reply
* Re: [PATCH v6 net-next 01/17] net: qualcomm: remove unnecessary includes
From: David Miller @ 2017-05-24 20:42 UTC (permalink / raw)
To: stefan.wahren
Cc: linux-serial, jslaby, gregkh, netdev, robh+dt, linux-kernel,
kubakici, mark.rutland, LinoSanfilippo, devicetree
In-Reply-To: <1746915556.279475.1495656327010@email.1und1.de>
From: Stefan Wahren <stefan.wahren@i2se.com>
Date: Wed, 24 May 2017 22:05:26 +0200 (CEST)
> AFAIK these ones above aren't necessary (no init, no kernel module,
> no kernel parameter, no kernel version) for this C file. So i will
> double check it.
You need the endianness translators like cpu_to_be32() or whatever,
so you need to figure out where you are getting that once these
explicit headers are removed.
And see, it's probably hidden inside of the private header's includes.
So we can't tell.
^ permalink raw reply
* Re: [PATCH 1/1] xilinx ps uart: Adding a kernel parameter for the number of xilinx ps uarts
From: Maarten Brock @ 2017-05-25 9:27 UTC (permalink / raw)
To: Michal Simek
Cc: Alan Cox, linux-serial-owner, gregkh, linux-kernel, Rob Herring,
Sam Povilus, linux-arm-kernel, linux-serial, jslaby,
soren.brinkmann
In-Reply-To: <b5a032c8-db86-c16b-750f-10aa2a132248@xilinx.com>
On 2017-05-24 18:09, Michal Simek wrote:
> On 24.5.2017 15:31, Alan Cox wrote:
>>> I am not saying that config option is perfect solution. It is at
>>> least
>>> aligned with 5 others serial drivers in the tree.
>>
>> And the fact people keep doing hacks jutifies continuing to make a
>> mess.
>> Especialy as in this case it's entirely theoretical. Nobody has
>> produced
>> actual hardware that hits the limit. Nobody has filed a bug, nobody is
>> impacted.
>
> This is the reason why we are talking about it how to do it right.
>
> With this ps uart it is not that easy because this is cadence RTL which
> is not in public IP database but the same think is with uartlite.
> Limit there is 16. If you really want that I can create that HW design
> which will require more than 16 uartlites in one design.
>
>
>> Creating extra CONFIG_ entries for junk like this is ridiculous, most
>> of
>> the others at least have the excuse of being old code.
>
> No doubt about it. I am just trying to find out what's the way you are
> suggesting.
A patch was already recently sent to this mailing list to add a CONFIG_
entry for the maximum number of uartlite devices. It is, as you say,
quite
easy to create a device with more than 16 uartlite devices. (It is
probably harder to create one with many Cadence RTL PS uarts since that
would require a license.)
IMHO it is quite normal for anyone using the uartlite to build his/her
own
kernel. and thus make config settings. But it is cumbersome to have to
modify the kernel sources. If the number of uartlites could be retrieved
from the device tree that would probably be even better if the price in
complexity and code size is reasonable.
Maarten
^ permalink raw reply
* [BUG] tty/serial: stty hangs for 30 seconds after interrupted transfer
From: Dmitry Artamonow @ 2017-05-25 13:22 UTC (permalink / raw)
To: Rob Herring, Greg Kroah-Hartman
Cc: Peter Hurley, linux-serial, linux-kernel, Uwe Kleine-König,
Fabio Estevam, Alan Cox
Hello.
While working on i.MX6-based board I found weird problem with serial ports.
When I do some write in serial port and then interrupt it prematurely with
Ctrl-C, following call to stty hangs for 30 seconds. Basic reproducing steps:
1. cat some_large_text_file.txt > /dev/ttymxc1
2. Press Ctrl-C
3. stty -F /dev/ttymxc1
If I send some more data over the port afterwards and don't interrupt it,
following call to stty doesn't hang.
Also issue can be reproduced by two small programs below. First one
forks itself and then child kills parent that writes to port after 1 second
timeout. After running first program, second hangs in close().
----- sendandkill.c
#include <fcntl.h>
#include <stdio.h>
#include <unistd.h>
char buffer[2400]; /* ~2 sec at 9600 baud */
int main(int argc, char **argv)
{
int fd, ret, i;
for(i = 0; i < sizeof(buffer); i++)
buffer[i] = '0' + (char)(i % 10);
ret = fork();
/* CHILD */
if (ret == 0) {
int parent_pid = getppid();
sleep(1);
kill(parent_pid, 9);
return 0;
}
/* PARENT */
fd = open("/dev/ttymxc1", O_RDWR);
write(fd, buffer, sizeof(buffer));
sleep(10);
close(fd);
return 0;
}
----- justopen.c
#include <fcntl.h>
int main(int argc, char **argv)
{
int fd = open("/dev/ttymxc1", O_RDWR);
close(fd);
return 0;
}
-----
Here's how it looks with the latest Linus's kernel (4.12.0-rc2+):
# uname -a
Linux imx6 4.12.0-rc2-00052-g56fff1b #7 SMP PREEMPT Wed May 24 15:03:17 MSK 2017 armv7l armv7l armv7l GNU/Linux
# stty -F -a /dev/ttymxc1
stty: -a: No such file or directory
# stty -a -F /dev/ttymxc1
speed 9600 baud; rows 0; columns 0; line = 0;
intr = ^C; quit = ^\; erase = ^?; kill = ^U; eof = ^D; eol = <undef>; eol2 = <undef>; swtch = <undef>; start = ^Q; stop = ^S; susp = ^Z;
rprnt = ^R; werase = ^W; lnext = ^V; discard = ^O; min = 1; time = 0;
-parenb -parodd -cmspar cs8 hupcl -cstopb cread clocal -crtscts
-ignbrk -brkint -ignpar -parmrk -inpck -istrip -inlcr -igncr icrnl ixon -ixoff -iuclc -ixany -imaxbel -iutf8
opost -olcuc -ocrnl onlcr -onocr -onlret -ofill -ofdel nl0 cr0 tab0 bs0 vt0 ff0
isig icanon iexten echo echoe echok -echonl -noflsh -xcase -tostop -echoprt echoctl echoke -flusho -extproc
# time ./justopen
real 0m0.045s
user 0m0.000s
sys 0m0.008s
# cat /proc/tty/driver/IMX-uart
serinfo:1.0 driver revision:
0: uart:IMX mmio:0x02020000 irq:27 tx:2601 rx:200 RTS|CTS|DTR|DSR|CD
1: uart:IMX mmio:0x021E8000 irq:75 tx:0 rx:0 DSR|CD
3: uart:IMX mmio:0x021F0000 irq:76 tx:0 rx:0 DSR|CD
4: uart:IMX mmio:0x021F4000 irq:77 tx:0 rx:0 DSR|CD
# strace -f -ttt ./sendandkill
1494138555.023757 execve("./sendandkill", ["./sendandkill"], [/* 13 vars */]) = 0
1494138555.042630 uname({sysname="Linux", nodename="imx6", ...}) = 0
1494138555.046067 brk(NULL) = 0x15a9000
1494138555.046198 brk(0x15a9d00) = 0x15a9d00
1494138555.046373 set_tls(0x15a94c0, 0x69fc0, 0, 0x6ac30, 0x6ac28) = 0
1494138555.053349 brk(0x15cad00) = 0x15cad00
1494138555.053496 brk(0x15cb000) = 0x15cb000
1494138555.053765 clone(child_stack=NULL, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x15a9068) = 510
1494138555.054141 open("/dev/ttymxc1", O_RDWR) = 3
1494138555.055081 write(3, "01234567890123456789012345678901"..., 2400) = 2400
1494138555.055331 rt_sigprocmask(SIG_BLOCK, [CHLD], [], 8) = 0
1494138555.055496 rt_sigaction(SIGCHLD, NULL, {SIG_DFL, [], 0}, 8) = 0
1494138555.055635 rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
1494138555.055768 nanosleep({10, 0}, strace: Process 510 attached
<unfinished ...>
[pid 510] 1494138555.056109 getppid() = 509
[pid 510] 1494138555.056227 rt_sigprocmask(SIG_BLOCK, [CHLD], [], 8) = 0
[pid 510] 1494138555.056382 rt_sigaction(SIGCHLD, NULL, {SIG_DFL, [], 0}, 8) = 0
[pid 510] 1494138555.056524 rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
[pid 510] 1494138555.056704 nanosleep({1, 0}, 0x7edbfc80) = 0
[pid 510] 1494138556.056986 kill(509, SIGKILL) = 0
[pid 510] 1494138556.060856 exit_group(0) = ?
[pid 509] 1494138556.069794 +++ killed by SIGKILL +++
1494138556.070003 +++ exited with 0 +++
Killed
# cat /proc/tty/driver/IMX-uart
serinfo:1.0 driver revision:
0: uart:IMX mmio:0x02020000 irq:27 tx:4465 rx:259 RTS|CTS|DTR|DSR|CD
1: uart:IMX mmio:0x021E8000 irq:75 tx:0 rx:0 DSR|CD
3: uart:IMX mmio:0x021F0000 irq:76 tx:0 rx:0 DSR|CD
4: uart:IMX mmio:0x021F4000 irq:77 tx:0 rx:0 DSR|CD
# time ./justopen
real 0m31.940s
user 0m0.000s
sys 0m0.007s
# strace -f -ttt ./justopen
1494138726.794853 execve("./justopen", ["./justopen"], [/* 13 vars */]) = 0
1494138726.795961 uname({sysname="Linux", nodename="imx6", ...}) = 0
1494138726.796176 brk(NULL) = 0x6a2000
1494138726.796297 brk(0x6a2d00) = 0x6a2d00
1494138726.796438 set_tls(0x6a24c0, 0x68fc0, 0, 0x69c30, 0x69c28) = 0
1494138726.796792 brk(0x6c3d00) = 0x6c3d00
1494138726.796940 brk(0x6c4000) = 0x6c4000
1494138726.797079 open("/dev/ttymxc1", O_RDWR) = 3
1494138726.798104 close(3) = 0
1494138757.400997 exit_group(0) = ?
1494138757.401223 +++ exited with 0 +++
# cat /proc/tty/driver/IMX-uart
serinfo:1.0 driver revision:
0: uart:IMX mmio:0x02020000 irq:27 tx:5475 rx:400 RTS|CTS|DTR|DSR|CD
1: uart:IMX mmio:0x021E8000 irq:75 tx:0 rx:0 DSR|CD
3: uart:IMX mmio:0x021F0000 irq:76 tx:0 rx:0 DSR|CD
4: uart:IMX mmio:0x021F4000 irq:77 tx:0 rx:0 DSR|CD
# echo -n "X" > /dev/ttymxc1
# cat /proc/tty/driver/IMX-uart
serinfo:1.0 driver revision:
0: uart:IMX mmio:0x02020000 irq:27 tx:5797 rx:400 RTS|CTS|DTR|DSR|CD
1: uart:IMX mmio:0x021E8000 irq:75 tx:2401 rx:0 DSR|CD
3: uart:IMX mmio:0x021F0000 irq:76 tx:0 rx:0 DSR|CD
4: uart:IMX mmio:0x021F4000 irq:77 tx:0 rx:0 DSR|CD
# time ./justopen
real 0m0.008s
user 0m0.000s
sys 0m0.007s
#
I located the place of the hang inside the tty_wait_until_sent() function.
Looks like it waits for UART to finish sending, but as the UART doesn't
actually send anything, it exits only after 30 second timeout.
After some digging I found that the problem seems to be caused by
commit 761ed4a94582 ("tty: serial_core: convert uart_close to use
tty_port_close"). Reverting this and related commits (a5a2b13074fd,
be2c92b8f164, 4dda864d7307) makes the problem go away.
Also note that statistics seems to be broken in the log above. After
running "sendandkill" it says that 0 bytes were sent while actually
about 970 bytes were transferred. And after sending one more character with
echo, it actually sends 2400+1 characters, i.e. resends the whole previously
interrupted buffer before sending 'X'. Reverts above fix this only partially.
UART doesn't resend previous characters anymore, but still statistics is
wrong about first interrupted transfer. But I suspect this could be a separate
problem in imx driver.
Any ideas how to fix this hang? I tried to put uart_flush_buffer() into
uart_close() before call to tty_port_close() and it kind of works, but
I'm not really sure whether it's a proper fix. Given my lack of experience
with TTY subsystem and level of its complexity I suspect that I can
easily miss something.
--
Best regards
Dmitry Artamonow
^ permalink raw reply
* Re: [PATCH 1/1] xilinx ps uart: Adding a kernel parameter for the number of xilinx ps uarts
From: Alan Cox @ 2017-05-25 13:29 UTC (permalink / raw)
To: Michal Simek
Cc: gregkh, linux-kernel, Rob Herring, Sam Povilus, linux-arm-kernel,
linux-serial, jslaby, soren.brinkmann
In-Reply-To: <b5a032c8-db86-c16b-750f-10aa2a132248@xilinx.com>
> Ok. Is there any problem if uart_register_driver is called for every
> instance separately with nr=1? This driver has major 0, minor 0. Based
If you are doing that level of code restructuring none that I am aware
of, and if you find one I'm happy to work on fixing it.
Alan
^ permalink raw reply
* Re: [BUG] tty/serial: stty hangs for 30 seconds after interrupted transfer
From: Dmitry Artamonow @ 2017-05-25 13:43 UTC (permalink / raw)
To: Rob Herring, Greg Kroah-Hartman
Cc: Peter Hurley, linux-serial, linux-kernel, Uwe Kleine-König,
Fabio Estevam, Alan Cox
In-Reply-To: <c87da751-0692-e381-be16-a2adf0f0104f@inbox.lv>
Update: the problem reproduces also on x86-64 with 8250 driver.
root@madhouse:~# uname -a
Linux madhouse 4.12.0-rc2+ #2 SMP Thu May 25 12:08:00 MSK 2017 x86_64 x86_64 x86_64 GNU/Linux
root@madhouse:~# time stty -F /dev/ttyS0
speed 9600 baud; line = 0;
-brkint -imaxbel
real 0m0.010s
user 0m0.011s
sys 0m0.000s
root@madhouse:~# cat blah > /dev/ttyS0
^C
root@madhouse:~# time stty -F /dev/ttyS0
speed 9600 baud; line = 0;
-brkint -imaxbel
real 0m30.732s
user 0m0.011s
sys 0m0.002s
root@madhouse:~#
--
Best regards
Dmitry Artamonow
^ permalink raw reply
* Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
From: Babu Moger @ 2017-05-25 14:51 UTC (permalink / raw)
To: Arnd Bergmann, Geert Uytterhoeven
Cc: David S. Miller, Peter Zijlstra, Ingo Molnar, sparclinux,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-Arch,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAK8P3a1XZGQk3b3q+JFJ+mLgw_upUdAtTM1VBjHZq5f2m-YdUg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Arnd,
Here is the draft patch. Some questions below.
On 5/24/2017 5:18 AM, Arnd Bergmann wrote:
> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
> <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
>>> Found this problem while enabling queued rwlock on SPARC.
>>> The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
>>> specific byte in qrwlock structure. Without this parameter,
>>> we clear the wrong byte. Here is the code.
>>>
>>> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>>> {
>>> return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>>> }
>>>
>>> Define CPU_BIG_ENDIAN for SPARC to fix it.
>>> --- a/arch/sparc/Kconfig
>>> +++ b/arch/sparc/Kconfig
>>> @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
>>> config ARCH_PROC_KCORE_TEXT
>>> def_bool y
>>>
>>> +config CPU_BIG_ENDIAN
>>> + bool
>>> + default y if SPARC
>> Nice catch!
>>
>> Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
>> architectures that may support both. And it was checked in platform code
>> and drivers only.
>> Hence the symbol is lacking from most architectures. Heck, even
>> architectures that support both may default to one endiannes, and declare
>> only the symbol for the other endianness:
>>
>> --- arch/alpha ---
>> --- arch/arc ---
>> arch/arc/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/arm ---
>> arch/arm/mm/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/arm64 ---
>> arch/arm64/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/blackfin ---
>> --- arch/c6x ---
>> arch/c6x/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/cris ---
>> --- arch/frv ---
>> --- arch/h8300 ---
>> --- arch/hexagon ---
>> --- arch/ia64 ---
>> --- arch/Kconfig ---
>> --- arch/m32r ---
>> arch/m32r/Kconfig:config CPU_LITTLE_ENDIAN
>> --- arch/m68k ---
>> --- arch/metag ---
>> --- arch/microblaze ---
>> --- arch/mips ---
>> arch/mips/Kconfig:config CPU_BIG_ENDIAN
>> arch/mips/Kconfig:config CPU_LITTLE_ENDIAN
>> --- arch/mn10300 ---
>> --- arch/nios2 ---
>> --- arch/openrisc ---
>> --- arch/parisc ---
>> --- arch/powerpc ---
>> arch/powerpc/platforms/Kconfig.cputype:config CPU_BIG_ENDIAN
>> arch/powerpc/platforms/Kconfig.cputype:config CPU_LITTLE_ENDIAN
>> --- arch/s390 ---
>> arch/s390/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/score ---
>> --- arch/sh ---
>> arch/sh/Kconfig.cpu:config CPU_LITTLE_ENDIAN
>> arch/sh/Kconfig.cpu:config CPU_BIG_ENDIAN
>> --- arch/sparc ---
>> --- arch/tile ---
>> --- arch/um ---
>> --- arch/unicore32 ---
>> --- arch/x86 ---
>> --- arch/xtensa ---
>>
>> However, there are already a few users in generic code, which are thus
>> broken on many platforms:
>>
>> drivers/of/base.c
>> drivers/of/fdt.c
>> drivers/tty/serial/earlycon.c
>> drivers/tty/serial/serial_core.c
>>
>> include/asm-generic/qrwlock.h is also generic, but depends on the
>> architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
>> (x86, and now sparc).
>>
>> I guess the time is ripe for adding (both) symbols to all architectures?
> Good idea. I think we can do most of this by adding a few lines to
> arch/Kconfig:
>
> config CPU_BIG_ENDIAN
> bool
>
> config CPU_LITTLE_ENDIAN
> def_bool !CPU_BIG_ENDIAN
>
> This way, we only need to add 'select CPU_BIG_ENDIAN' to the
> architectures that are always big-endian, and we don't need to
> change anything for the ones that have a single 'CPU_BIG_ENDIAN'
> option.
>
> The three architectures that have a 'choice' statement (mips, ppc and
> sh) will have to convert, and m32r will have to replace the
what to you mean by "(mips, ppc andsh) will have to convert"? Do you
expect any changes here?
> option with the opposite one, which could break 'make oldconfig',
> but nobody really cares about m32r any more.
>
> Arnd
Here is the proposed draft patch
======================================
---
arch/Kconfig | 6 ++++++
arch/frv/Kconfig | 1 +
arch/h8300/Kconfig | 1 +
arch/m32r/Kconfig | 6 +++---
arch/m68k/Kconfig | 1 +
arch/openrisc/Kconfig | 1 +
arch/parisc/Kconfig | 1 +
arch/s390/Kconfig | 1 +
arch/sparc/Kconfig | 1 +
9 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 6c00e5b..19fcafd 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -867,4 +867,10 @@ config STRICT_MODULE_RWX
config ARCH_WANT_RELAX_ORDER
bool
+config CPU_BIG_ENDIAN
+ bool
+
+config CPU_LITTLE_ENDIAN
+ def_bool !CPU_BIG_ENDIAN
+
source "kernel/gcov/Kconfig"
diff --git a/arch/frv/Kconfig b/arch/frv/Kconfig
index eefd9a4..db258a4 100644
--- a/arch/frv/Kconfig
+++ b/arch/frv/Kconfig
@@ -16,6 +16,7 @@ config FRV
select OLD_SIGACTION
select HAVE_DEBUG_STACKOVERFLOW
select ARCH_NO_COHERENT_DMA_MMAP
+ select CPU_BIG_ENDIAN
config ZONE_DMA
bool
diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index 3ae8525..22ebf28 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -22,6 +22,7 @@ config H8300
select HAVE_ARCH_KGDB
select HAVE_ARCH_HASH
select CPU_NO_EFFICIENT_FFS
+ select CPU_BIG_ENDIAN
config RWSEM_GENERIC_SPINLOCK
def_bool y
diff --git a/arch/m32r/Kconfig b/arch/m32r/Kconfig
index 9547446..d57e37b 100644
--- a/arch/m32r/Kconfig
+++ b/arch/m32r/Kconfig
@@ -193,9 +193,9 @@ config TIMER_DIVIDE
int "Timer divider (integer)"
default "128"
-config CPU_LITTLE_ENDIAN
- bool "Generate little endian code"
- default n
+config CPU_BIG_ENDIAN
+ bool "Generate big endian code"
+ default y
config MEMORY_START
hex "Physical memory start address (hex)"
diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index d140206..b44579b 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -22,6 +22,7 @@ config M68K
select MODULES_USE_ELF_RELA
select OLD_SIGSUSPEND3
select OLD_SIGACTION
+ select CPU_BIG_ENDIAN
config RWSEM_GENERIC_SPINLOCK
bool
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 1e95920..d772983 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -28,6 +28,7 @@ config OPENRISC
select OR1K_PIC
select CPU_NO_EFFICIENT_FFS if !OPENRISC_HAVE_INST_FF1
select NO_BOOTMEM
+ select CPU_BIG_ENDIAN
config MMU
def_bool y
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 531da9e..7a8ec28 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -40,6 +40,7 @@ config PARISC
select GENERIC_CLOCKEVENTS
select ARCH_NO_COHERENT_DMA_MMAP
select CPU_NO_EFFICIENT_FFS
+ select CPU_BIG_ENDIAN
help
The PA-RISC microprocessor is designed by Hewlett-Packard and
used
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index e161faf..169e50b 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -177,6 +177,7 @@ config S390
select ARCH_HAS_SCALED_CPUTIME
select VIRT_TO_BUS
select HAVE_NMI
+ select CPU_BIG_ENDIAN
config SCHED_OMIT_FRAME_POINTER
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index c5b5a7b..7a28e8a 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -45,6 +45,7 @@ config SPARC
select CPU_NO_EFFICIENT_FFS
select LOCKDEP_SMALL if LOCKDEP
select ARCH_WANT_RELAX_ORDER
+ select CPU_BIG_ENDIAN
config SPARC32
def_bool !64BIT
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 1/1] xilinx ps uart: Adding a kernel parameter for the number of xilinx ps uarts
From: Michal Simek @ 2017-05-25 15:33 UTC (permalink / raw)
To: Alan Cox, Michal Simek
Cc: Rob Herring, Sam Povilus, gregkh, jslaby, soren.brinkmann,
linux-serial, linux-arm-kernel, linux-kernel
In-Reply-To: <20170525142920.1d662467@alans-desktop>
On 25.5.2017 15:29, Alan Cox wrote:
>> Ok. Is there any problem if uart_register_driver is called for every
>> instance separately with nr=1? This driver has major 0, minor 0. Based
>
> If you are doing that level of code restructuring none that I am aware
> of, and if you find one I'm happy to work on fixing it.
ok. Let me try to play with it and see if this is working.
M
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox