All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robherring2@gmail.com>
To: Lyra Zhang <zhang.lyra@gmail.com>
Cc: "Chunyan Zhang" <chunyan.zhang@spreadtrum.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"One Thousand Gnomes" <gnomes@lxorguk.ukuu.org.uk>,
	"Mark Brown" <broonie@kernel.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Pawel Moll" <pawel.moll@arm.com>,
	"Ian Campbell" <ijc+devicetree@hellion.org.uk>,
	"Kumar Gala" <galak@codeaurora.org>,
	"Will Deacon" <will.deacon@arm.com>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Jiri Slaby" <jslaby@suse.cz>,
	"Jason Cooper" <jason@lakedaemon.net>,
	"Heiko Stübner" <heiko@sntech.de>,
	"Florian Vaussard" <florian.vaussard@epfl.ch>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Robert Richter" <rrichter@cavium.com>,
	"Hayato Suzuki" <hytszk@gmail.com>,
	"Grant Likely" <grant.likely@linaro.org>,
	"Antony Pavlov" <antonynpavlov@gmail.com>,
	"Joel Schopp" <Joel.Schopp@amd.com>,
	"Suravee Suthikulanit" <Suravee.Suthikulpanit@amd.com>,
	"Shawn Guo" <shawn.guo@linaro.org>,
	lea.yan@linaro.org,
	"Jorge Ramirez-Ortiz" <jorge.ramirez-ortiz@linaro.org>,
	"Lee Jones" <lee.jones@linaro.org>,
	"Orson Zhai" <orsonzhai@gmail.com>,
	"geng.ren@spreadtrum.com" <geng.ren@spreadtrum.com>,
	"zhizhou.zhang" <zhizhou.zhang@spreadtrum.com>,
	"lanqing.liu@spreadtrum.com" <lanqing.liu@spreadtrum.com>,
	"Wei Qiao (乔伟)" <wei.qiao@spreadtrum.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-api@vger.kernel.org" <linux-api@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>
Subject: Re: [PATCH v5 5/5] tty/serial: Add Spreadtrum sc9836-uart driver support
Date: Mon, 19 Jan 2015 08:11:20 -0600	[thread overview]
Message-ID: <CAL_JsqLTArm1v8ka39_KLq2jMKkK2ZXQ6-an=JSWVgTK1LWmOg@mail.gmail.com> (raw)
In-Reply-To: <CAAfSe-vqqpvbYe0bEeD-3_QsLYz4r7pxMuA1-L4cYRw9TY=NgA@mail.gmail.com>

On Mon, Jan 19, 2015 at 3:55 AM, Lyra Zhang <zhang.lyra@gmail.com> wrote:
> On Sat, Jan 17, 2015 at 12:41 AM, Rob Herring <robherring2@gmail.com> wrote:
>> On Fri, Jan 16, 2015 at 4:00 AM, Chunyan Zhang
>> <chunyan.zhang@spreadtrum.com> wrote:
>>> Add a full sc9836-uart driver for SC9836 SoC which is based on the
>>> spreadtrum sharkl64 platform.
>>> This driver also support earlycon.
>>> This patch also replaced the spaces between the macros and their
>>> values with the tabs in serial_core.h

[...]

>>> +static int __init sprd_serial_init(void)
>>> +{
>>> +       int ret = 0;
>>> +
>>> +       ret = uart_register_driver(&sprd_uart_driver);
>>
>> This can be done in probe now. Then you can use module_platform_driver().
>>
>
> Question:
> 1. there are 4 uart ports configured in dt for sprd_serial, so probe
> will be called 4 times, but uart_register_driver only needs to be
> called one time, so can we use uart_driver.state to check if
> uart_register_driver has already been called ?

Yes.

> 2. if I use module_platform_driver() instead of
> module_init(sprd_serial_init)  and  module_exit(sprd_serial_exit) , I
> must move uart_unregister_driver() which is now processed in
> sprd_serial_exit() to sprd_remove(), there is a similar problem with
> probe(), sprd_remove() will also be called 4 times, and actually it
> should be called only one time. How can we deal with this case?

Look at pl01x or Samsung UART drivers which have done this conversion.

> 3. for the second question, we can check the platform_device->id, if
> it is equal to the index of last port (e.g. 4 for this case), then
> uart_unregister_driver() can be called. Does it work correctly? since
> for this case, we must keep the order of releasing ports.

The id will not be the line index in the DT case. I don't think you
can guarantee the order either.

It would be better to make uart_{un}register_driver deal with being
called multiple times so drivers don't have to deal with getting this
correct. I'm not sure if that is feasible though.

Rob

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robherring2@gmail.com>
To: Lyra Zhang <zhang.lyra@gmail.com>
Cc: "Chunyan Zhang" <chunyan.zhang@spreadtrum.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"One Thousand Gnomes" <gnomes@lxorguk.ukuu.org.uk>,
	"Mark Brown" <broonie@kernel.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Pawel Moll" <pawel.moll@arm.com>,
	"Ian Campbell" <ijc+devicetree@hellion.org.uk>,
	"Kumar Gala" <galak@codeaurora.org>,
	"Will Deacon" <will.deacon@arm.com>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Jiri Slaby" <jslaby@suse.cz>,
	"Jason Cooper" <jason@lakedaemon.net>,
	"Heiko Stübner" <heiko@sntech.de>,
	"Florian Vaussard" <florian.vaussard@epfl.ch>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Robert Richter" <rrichter@cavium.com>,
	"Hayato Suzuki" <hytszk@gmail.com>,
	"Grant Likely" <grant.likely@linaro.org>,
	"Antony Pavlov" <antonynpavlov@gmail.com>,
	"Joel Schopp" <Joel.Schopp@amd.com>
Subject: Re: [PATCH v5 5/5] tty/serial: Add Spreadtrum sc9836-uart driver support
Date: Mon, 19 Jan 2015 08:11:20 -0600	[thread overview]
Message-ID: <CAL_JsqLTArm1v8ka39_KLq2jMKkK2ZXQ6-an=JSWVgTK1LWmOg@mail.gmail.com> (raw)
In-Reply-To: <CAAfSe-vqqpvbYe0bEeD-3_QsLYz4r7pxMuA1-L4cYRw9TY=NgA@mail.gmail.com>

On Mon, Jan 19, 2015 at 3:55 AM, Lyra Zhang <zhang.lyra@gmail.com> wrote:
> On Sat, Jan 17, 2015 at 12:41 AM, Rob Herring <robherring2@gmail.com> wrote:
>> On Fri, Jan 16, 2015 at 4:00 AM, Chunyan Zhang
>> <chunyan.zhang@spreadtrum.com> wrote:
>>> Add a full sc9836-uart driver for SC9836 SoC which is based on the
>>> spreadtrum sharkl64 platform.
>>> This driver also support earlycon.
>>> This patch also replaced the spaces between the macros and their
>>> values with the tabs in serial_core.h

[...]

>>> +static int __init sprd_serial_init(void)
>>> +{
>>> +       int ret = 0;
>>> +
>>> +       ret = uart_register_driver(&sprd_uart_driver);
>>
>> This can be done in probe now. Then you can use module_platform_driver().
>>
>
> Question:
> 1. there are 4 uart ports configured in dt for sprd_serial, so probe
> will be called 4 times, but uart_register_driver only needs to be
> called one time, so can we use uart_driver.state to check if
> uart_register_driver has already been called ?

Yes.

> 2. if I use module_platform_driver() instead of
> module_init(sprd_serial_init)  and  module_exit(sprd_serial_exit) , I
> must move uart_unregister_driver() which is now processed in
> sprd_serial_exit() to sprd_remove(), there is a similar problem with
> probe(), sprd_remove() will also be called 4 times, and actually it
> should be called only one time. How can we deal with this case?

Look at pl01x or Samsung UART drivers which have done this conversion.

> 3. for the second question, we can check the platform_device->id, if
> it is equal to the index of last port (e.g. 4 for this case), then
> uart_unregister_driver() can be called. Does it work correctly? since
> for this case, we must keep the order of releasing ports.

The id will not be the line index in the DT case. I don't think you
can guarantee the order either.

It would be better to make uart_{un}register_driver deal with being
called multiple times so drivers don't have to deal with getting this
correct. I'm not sure if that is feasible though.

Rob

WARNING: multiple messages have this Message-ID (diff)
From: robherring2@gmail.com (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 5/5] tty/serial: Add Spreadtrum sc9836-uart driver support
Date: Mon, 19 Jan 2015 08:11:20 -0600	[thread overview]
Message-ID: <CAL_JsqLTArm1v8ka39_KLq2jMKkK2ZXQ6-an=JSWVgTK1LWmOg@mail.gmail.com> (raw)
In-Reply-To: <CAAfSe-vqqpvbYe0bEeD-3_QsLYz4r7pxMuA1-L4cYRw9TY=NgA@mail.gmail.com>

On Mon, Jan 19, 2015 at 3:55 AM, Lyra Zhang <zhang.lyra@gmail.com> wrote:
> On Sat, Jan 17, 2015 at 12:41 AM, Rob Herring <robherring2@gmail.com> wrote:
>> On Fri, Jan 16, 2015 at 4:00 AM, Chunyan Zhang
>> <chunyan.zhang@spreadtrum.com> wrote:
>>> Add a full sc9836-uart driver for SC9836 SoC which is based on the
>>> spreadtrum sharkl64 platform.
>>> This driver also support earlycon.
>>> This patch also replaced the spaces between the macros and their
>>> values with the tabs in serial_core.h

[...]

>>> +static int __init sprd_serial_init(void)
>>> +{
>>> +       int ret = 0;
>>> +
>>> +       ret = uart_register_driver(&sprd_uart_driver);
>>
>> This can be done in probe now. Then you can use module_platform_driver().
>>
>
> Question:
> 1. there are 4 uart ports configured in dt for sprd_serial, so probe
> will be called 4 times, but uart_register_driver only needs to be
> called one time, so can we use uart_driver.state to check if
> uart_register_driver has already been called ?

Yes.

> 2. if I use module_platform_driver() instead of
> module_init(sprd_serial_init)  and  module_exit(sprd_serial_exit) , I
> must move uart_unregister_driver() which is now processed in
> sprd_serial_exit() to sprd_remove(), there is a similar problem with
> probe(), sprd_remove() will also be called 4 times, and actually it
> should be called only one time. How can we deal with this case?

Look at pl01x or Samsung UART drivers which have done this conversion.

> 3. for the second question, we can check the platform_device->id, if
> it is equal to the index of last port (e.g. 4 for this case), then
> uart_unregister_driver() can be called. Does it work correctly? since
> for this case, we must keep the order of releasing ports.

The id will not be the line index in the DT case. I don't think you
can guarantee the order either.

It would be better to make uart_{un}register_driver deal with being
called multiple times so drivers don't have to deal with getting this
correct. I'm not sure if that is feasible though.

Rob

  reply	other threads:[~2015-01-19 14:11 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <sc9836-v5>
2015-01-16 10:00 ` [PATCH v5 0/5] Add Spreadtrum Sharkl64 Platform support Chunyan Zhang
2015-01-16 10:00   ` Chunyan Zhang
2015-01-16 10:00   ` Chunyan Zhang
2015-01-16 10:00   ` [PATCH v5 1/5] Documentation: DT: Renamed of-serial.txt to 8250.txt Chunyan Zhang
2015-01-16 10:00     ` Chunyan Zhang
2015-01-16 10:00     ` Chunyan Zhang
2015-01-16 14:11     ` Rob Herring
2015-01-16 14:11       ` Rob Herring
2015-01-16 14:11       ` Rob Herring
2015-01-16 10:00   ` [PATCH v5 2/5] Documentation: DT: Add bindings for Spreadtrum SoC Platform Chunyan Zhang
2015-01-16 10:00     ` Chunyan Zhang
2015-01-16 10:00     ` Chunyan Zhang
2015-01-16 10:21     ` Mark Rutland
2015-01-16 10:21       ` Mark Rutland
2015-01-16 10:21       ` Mark Rutland
2015-01-16 12:53       ` Lyra Zhang
2015-01-16 12:53         ` Lyra Zhang
2015-01-16 12:53         ` Lyra Zhang
2015-01-16 14:11         ` Mark Rutland
2015-01-16 14:11           ` Mark Rutland
2015-01-16 14:11           ` Mark Rutland
2015-01-17  8:10           ` Orson Zhai
2015-01-17  8:10             ` Orson Zhai
2015-01-17  8:10             ` Orson Zhai
2015-01-16 10:00   ` [PATCH v5 3/5] arm64: dts: Add support for Spreadtrum SC9836 SoC in dts and Makefile Chunyan Zhang
2015-01-16 10:00     ` Chunyan Zhang
2015-01-16 10:00     ` Chunyan Zhang
2015-01-16 10:35     ` Mark Rutland
2015-01-16 10:35       ` Mark Rutland
2015-01-16 10:35       ` Mark Rutland
2015-01-16 12:49       ` Orson Zhai
2015-01-16 12:49         ` Orson Zhai
2015-01-16 12:49         ` Orson Zhai
2015-01-16 14:09         ` Mark Rutland
2015-01-16 14:09           ` Mark Rutland
2015-01-16 14:09           ` Mark Rutland
2015-01-17  8:47           ` Orson Zhai
2015-01-17  8:47             ` Orson Zhai
2015-01-17  8:47             ` Orson Zhai
2015-01-16 10:00   ` [PATCH v5 4/5] arm64: Add support for Spreadtrum's Sharkl64 Platform in Kconfig and defconfig Chunyan Zhang
2015-01-16 10:00     ` Chunyan Zhang
2015-01-16 10:00     ` Chunyan Zhang
2015-01-16 10:48     ` Mark Rutland
2015-01-16 10:48       ` Mark Rutland
2015-01-16 10:48       ` Mark Rutland
2015-01-16 11:50       ` Lyra Zhang
2015-01-16 11:50         ` Lyra Zhang
2015-01-16 11:50         ` Lyra Zhang
2015-01-16 10:00   ` [PATCH v5 5/5] tty/serial: Add Spreadtrum sc9836-uart driver support Chunyan Zhang
2015-01-16 10:00     ` Chunyan Zhang
2015-01-16 10:00     ` Chunyan Zhang
2015-01-16 10:26     ` Arnd Bergmann
2015-01-16 10:26       ` Arnd Bergmann
2015-01-16 11:02     ` Baruch Siach
2015-01-16 11:02       ` Baruch Siach
2015-01-16 11:02       ` Baruch Siach
2015-01-16 15:20     ` Peter Hurley
2015-01-16 15:20       ` Peter Hurley
2015-01-16 15:20       ` Peter Hurley
2015-01-20 12:11       ` Orson Zhai
2015-01-20 12:11         ` Orson Zhai
2015-01-20 13:39         ` Peter Hurley
2015-01-20 13:39           ` Peter Hurley
2015-01-16 16:41     ` Rob Herring
2015-01-16 16:41       ` Rob Herring
2015-01-16 16:41       ` Rob Herring
2015-01-19  9:55       ` Lyra Zhang
2015-01-19  9:55         ` Lyra Zhang
2015-01-19  9:55         ` Lyra Zhang
2015-01-19 14:11         ` Rob Herring [this message]
2015-01-19 14:11           ` Rob Herring
2015-01-19 14:11           ` Rob Herring
2015-01-20  7:37           ` Lyra Zhang
2015-01-20  7:37             ` Lyra Zhang
2015-01-20  7:37             ` Lyra Zhang
2015-01-20  8:41             ` Orson Zhai
2015-01-20  8:41               ` Orson Zhai
2015-01-20  8:41               ` Orson Zhai
2015-01-20 20:17             ` Rob Herring
2015-01-20 20:17               ` Rob Herring
2015-01-20 20:17               ` Rob Herring

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to='CAL_JsqLTArm1v8ka39_KLq2jMKkK2ZXQ6-an=JSWVgTK1LWmOg@mail.gmail.com' \
    --to=robherring2@gmail.com \
    --cc=Joel.Schopp@amd.com \
    --cc=Suravee.Suthikulpanit@amd.com \
    --cc=andrew@lunn.ch \
    --cc=antonynpavlov@gmail.com \
    --cc=arnd@arndb.de \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=chunyan.zhang@spreadtrum.com \
    --cc=devicetree@vger.kernel.org \
    --cc=florian.vaussard@epfl.ch \
    --cc=galak@codeaurora.org \
    --cc=geng.ren@spreadtrum.com \
    --cc=gnomes@lxorguk.ukuu.org.uk \
    --cc=grant.likely@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=heiko@sntech.de \
    --cc=hytszk@gmail.com \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=jason@lakedaemon.net \
    --cc=jorge.ramirez-ortiz@linaro.org \
    --cc=jslaby@suse.cz \
    --cc=lanqing.liu@spreadtrum.com \
    --cc=lea.yan@linaro.org \
    --cc=lee.jones@linaro.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=orsonzhai@gmail.com \
    --cc=pawel.moll@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=rrichter@cavium.com \
    --cc=shawn.guo@linaro.org \
    --cc=wei.qiao@spreadtrum.com \
    --cc=will.deacon@arm.com \
    --cc=zhang.lyra@gmail.com \
    --cc=zhizhou.zhang@spreadtrum.com \
    /path/to/YOUR_REPLY

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

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