* [RFC PATCH 0/3] serial: uartps: Add run time support for more IPs than hardcoded 2
@ 2018-04-26 14:08 ` Michal Simek
0 siblings, 0 replies; 22+ messages in thread
From: Michal Simek @ 2018-04-26 14:08 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
this series is trying to address discussion I had with Alan in past
https://patchwork.kernel.org/patch/9738445/.
It is moving uart_register_driver() to probe function like it is done in
pl011 driver.
And also introducing new function for alias compatibility checking to
resolve cases where some IPs have alias and some of them not.
This case is detected in pl011_probe_dt_alias() but not properly solved.
Also keep status of free ids/minor numbers in bitmap to know what's the
next unallocated number.
The same solution can be used in pl011, uart16550 and uartlite to really
get unified kernel.
Tested on these use cases:
Notes:
ff000000 is the first PS uart listed in dtsi
ff010000 is the second PS uart listed in dtsi
Standard zcu102 setting
serial0 = &uart0;
serial1 = &uart1;
[ 0.196628] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
base_baud = 6250000) is a xuartps
[ 0.642542] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
base_baud = 6250000) is a xuartps
Swapped zcu102 setting
serial0 = &uart1;
serial1 = &uart0;
[ 0.196472] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
base_baud = 6250000) is a xuartps
[ 0.196824] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
base_baud = 6250000) is a xuartps
uart0 on alias higher then MAX uart ports
serial0 = &uart1;
serial200 = &uart0;
[ 0.176793] ff000000.serial: ttyPS200 at MMIO 0xff000000 (irq = 33,
base_baud = 6250000) is a xuartps
[ 0.177288] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
base_baud = 6250000) is a xuartps
Both uarts on higher aliases
serial1 = &uart1;
serial2 = &uart0;
[ 0.196470] ff000000.serial: ttyPS2 at MMIO 0xff000000 (irq = 33,
base_baud = 6250000) is a xuartps
[ 0.196823] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
base_baud = 6250000) is a xuartps
uart0 not listed but it is probed first that's why should be ttyPS0 but
there is uart1 via alias
serial0 = &uart1;
[ 0.176656] xuartps ff000000.serial: No serial alias passed. Using
the first free id
[ 0.176671] xuartps ff000000.serial: Validate id 0
[ 0.176680] xuartps ff000000.serial: The empty id is 0
[ 0.176692] xuartps ff000000.serial: ID 0 already taken - skipped
[ 0.176701] xuartps ff000000.serial: Validate id 1
[ 0.176710] xuartps ff000000.serial: The empty id is 1
[ 0.176719] xuartps ff000000.serial: Selected ID 1 allocation passed
[ 0.176760] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
base_baud = 6250000) is a xuartps
[ 0.177104] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
base_baud = 6250000) is a xuartps
uart0 not listed but it is probed first that's why should be ttyPS0
serial1 = &uart1;
[ 0.176661] xuartps ff000000.serial: No serial alias passed. Using
the first free id
[ 0.176676] xuartps ff000000.serial: Validate id 0
[ 0.176686] xuartps ff000000.serial: The empty id is 0
[ 0.176696] xuartps ff000000.serial: Selected ID 0 allocation passed
[ 0.176737] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
base_baud = 6250000) is a xuartps
[ 0.177069] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
base_baud = 6250000) is a xuartps
uarts not listed in aliases list
[ 0.176673] xuartps ff000000.serial: No serial alias passed. Using
the first free id
[ 0.176687] xuartps ff000000.serial: Validate id 0
[ 0.176697] xuartps ff000000.serial: The empty id is 0
[ 0.176707] xuartps ff000000.serial: Selected ID 0 allocation passed
[ 0.176746] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
base_baud = 6250000) is a xuartps
[ 0.177057] xuartps ff010000.serial: No serial alias passed. Using
the first free id
[ 0.177070] xuartps ff010000.serial: Validate id 0
[ 0.177079] xuartps ff010000.serial: The empty id is 0
[ 0.177089] xuartps ff010000.serial: Selected ID 0 allocation failed
[ 0.177098] xuartps ff010000.serial: Validate id 1
[ 0.177107] xuartps ff010000.serial: The empty id is 1
[ 0.177116] xuartps ff010000.serial: Selected ID 1 allocation passed
[ 0.177149] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
base_baud = 6250000) is a xuartps
Thanks,
Michal
Michal Simek (3):
of: base: Introduce of_alias_check_id() to check alias IDs
serial: uartps: Move register to probe based on run time detection
serial: uartps: Change uart ports allocation
drivers/of/base.c | 49 +++++++++++
drivers/tty/serial/xilinx_uartps.c | 128 +++++++++++++++++++++++------
include/linux/of.h | 2 +
3 files changed, 153 insertions(+), 26 deletions(-)
--
2.17.0
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH 1/3] of: base: Introduce of_alias_check_id() to check alias IDs
2018-04-26 14:08 ` Michal Simek
(?)
@ 2018-04-26 14:08 ` Michal Simek
2018-04-27 2:39 ` Rob Herring
-1 siblings, 1 reply; 22+ messages in thread
From: Michal Simek @ 2018-04-26 14:08 UTC (permalink / raw)
To: linux-kernel, monstr, gnomes, Alexander Graf
Cc: devicetree, Rob Herring, Frank Rowand
The function travers the lookup table to check if the request alias
id is compatible with the device driver match structure.
This function will be used by serial drivers to check if requested alias
is allocated or free to use.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/of/base.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/of.h | 2 ++
2 files changed, 51 insertions(+)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 848f549164cd..382de01acc72 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1892,6 +1892,55 @@ int of_alias_get_id(struct device_node *np, const char *stem)
}
EXPORT_SYMBOL_GPL(of_alias_get_id);
+/**
+ * of_alias_check_id - Check alias id for the give compatibility
+ * @matches: Array of of device match structures to search in
+ * @stem: Alias stem of the given device_node
+ * @id: Alias ID for checking
+ *
+ * The function travers the lookup table to check if the request alias id
+ * is compatible with the device driver match structure
+ *
+ * Return true if ID is allocated, return false if not
+ */
+bool of_alias_check_id(const struct of_device_id *matches, const char *stem,
+ int id)
+{
+ struct alias_prop *app;
+ bool ret = false;
+
+ mutex_lock(&of_mutex);
+ pr_debug("%s: Looking for stem: %s, id: %d\n", __func__, stem, id);
+ list_for_each_entry(app, &aliases_lookup, link) {
+ pr_debug("%s: stem: %s, id: %d\n",
+ __func__, app->stem, app->id);
+
+ if (strcmp(app->stem, stem) != 0) {
+ pr_debug("%s: stem comparison doesn't passed %s\n",
+ __func__, app->stem);
+ continue;
+ }
+
+ if (app->id != id) {
+ pr_debug("%s: id comparison doesn't passed %d\n",
+ __func__, app->id);
+ continue;
+ }
+
+ if (of_match_node(matches, app->np)) {
+ pr_debug("%s: Allocated ID %d\n", __func__, app->id);
+ ret = true;
+ break;
+ }
+ pr_debug("%s: Free ID %d\n", __func__, app->id);
+ break;
+ }
+ mutex_unlock(&of_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(of_alias_check_id);
+
/**
* of_alias_get_highest_id - Get highest alias id for the given stem
* @stem: Alias stem to be examined
diff --git a/include/linux/of.h b/include/linux/of.h
index 4d25e4f952d9..a18a390a6129 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -387,6 +387,8 @@ extern int of_phandle_iterator_args(struct of_phandle_iterator *it,
extern void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align));
extern int of_alias_get_id(struct device_node *np, const char *stem);
extern int of_alias_get_highest_id(const char *stem);
+extern bool of_alias_check_id(const struct of_device_id *matches,
+ const char *stem, int id);
extern int of_machine_is_compatible(const char *compat);
--
2.17.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [RFC PATCH 1/3] of: base: Introduce of_alias_check_id() to check alias IDs
2018-04-26 14:08 ` [RFC PATCH 1/3] of: base: Introduce of_alias_check_id() to check alias IDs Michal Simek
@ 2018-04-27 2:39 ` Rob Herring
2018-04-27 6:10 ` Michal Simek
0 siblings, 1 reply; 22+ messages in thread
From: Rob Herring @ 2018-04-27 2:39 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, Michal Simek, One Thousand Gnomes, Alexander Graf,
devicetree, Frank Rowand
On Thu, Apr 26, 2018 at 9:08 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> The function travers the lookup table to check if the request alias
> id is compatible with the device driver match structure.
> This function will be used by serial drivers to check if requested alias
> is allocated or free to use.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> drivers/of/base.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/of.h | 2 ++
> 2 files changed, 51 insertions(+)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 848f549164cd..382de01acc72 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1892,6 +1892,55 @@ int of_alias_get_id(struct device_node *np, const char *stem)
> }
> EXPORT_SYMBOL_GPL(of_alias_get_id);
>
> +/**
> + * of_alias_check_id - Check alias id for the give compatibility
> + * @matches: Array of of device match structures to search in
> + * @stem: Alias stem of the given device_node
> + * @id: Alias ID for checking
> + *
> + * The function travers the lookup table to check if the request alias id
> + * is compatible with the device driver match structure
> + *
> + * Return true if ID is allocated, return false if not
> + */
> +bool of_alias_check_id(const struct of_device_id *matches, const char *stem,
> + int id)
Wouldn't it be simpler to just return a bitmap of all allocated ids
that match rather than trying to build that up 1 bit at a time?
Rob
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 1/3] of: base: Introduce of_alias_check_id() to check alias IDs
2018-04-27 2:39 ` Rob Herring
@ 2018-04-27 6:10 ` Michal Simek
0 siblings, 0 replies; 22+ messages in thread
From: Michal Simek @ 2018-04-27 6:10 UTC (permalink / raw)
To: Rob Herring, Michal Simek
Cc: linux-kernel, Michal Simek, One Thousand Gnomes, Alexander Graf,
devicetree, Frank Rowand
On 27.4.2018 04:39, Rob Herring wrote:
> On Thu, Apr 26, 2018 at 9:08 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> The function travers the lookup table to check if the request alias
>> id is compatible with the device driver match structure.
>> This function will be used by serial drivers to check if requested alias
>> is allocated or free to use.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> drivers/of/base.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/of.h | 2 ++
>> 2 files changed, 51 insertions(+)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 848f549164cd..382de01acc72 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -1892,6 +1892,55 @@ int of_alias_get_id(struct device_node *np, const char *stem)
>> }
>> EXPORT_SYMBOL_GPL(of_alias_get_id);
>>
>> +/**
>> + * of_alias_check_id - Check alias id for the give compatibility
>> + * @matches: Array of of device match structures to search in
>> + * @stem: Alias stem of the given device_node
>> + * @id: Alias ID for checking
>> + *
>> + * The function travers the lookup table to check if the request alias id
>> + * is compatible with the device driver match structure
>> + *
>> + * Return true if ID is allocated, return false if not
>> + */
>> +bool of_alias_check_id(const struct of_device_id *matches, const char *stem,
>> + int id)
>
> Wouldn't it be simpler to just return a bitmap of all allocated ids
> that match rather than trying to build that up 1 bit at a time?
Is alias list stable or can dt overlay change it?
What should be the expected flow? Find out maximum number of aliases of
the same kind and allocate bitmap and return it with length.
Anyway if you look at that patches I sent then I call in the driver
of_alias_get_highest_id("serial") which doesn't take care if alias match
with actual driver. It means having information about max alias ID which
match actual driver that would be helpful but I am not quite sure what
should be the flow.
Any link to similar function would be good to understand how the flow is
supposed to work.
Thanks,
Michal
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 1/3] of: base: Introduce of_alias_check_id() to check alias IDs
@ 2018-04-27 6:10 ` Michal Simek
0 siblings, 0 replies; 22+ messages in thread
From: Michal Simek @ 2018-04-27 6:10 UTC (permalink / raw)
To: Rob Herring, Michal Simek
Cc: linux-kernel, Michal Simek, One Thousand Gnomes, Alexander Graf,
devicetree, Frank Rowand
On 27.4.2018 04:39, Rob Herring wrote:
> On Thu, Apr 26, 2018 at 9:08 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> The function travers the lookup table to check if the request alias
>> id is compatible with the device driver match structure.
>> This function will be used by serial drivers to check if requested alias
>> is allocated or free to use.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> drivers/of/base.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/of.h | 2 ++
>> 2 files changed, 51 insertions(+)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 848f549164cd..382de01acc72 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -1892,6 +1892,55 @@ int of_alias_get_id(struct device_node *np, const char *stem)
>> }
>> EXPORT_SYMBOL_GPL(of_alias_get_id);
>>
>> +/**
>> + * of_alias_check_id - Check alias id for the give compatibility
>> + * @matches: Array of of device match structures to search in
>> + * @stem: Alias stem of the given device_node
>> + * @id: Alias ID for checking
>> + *
>> + * The function travers the lookup table to check if the request alias id
>> + * is compatible with the device driver match structure
>> + *
>> + * Return true if ID is allocated, return false if not
>> + */
>> +bool of_alias_check_id(const struct of_device_id *matches, const char *stem,
>> + int id)
>
> Wouldn't it be simpler to just return a bitmap of all allocated ids
> that match rather than trying to build that up 1 bit at a time?
Is alias list stable or can dt overlay change it?
What should be the expected flow? Find out maximum number of aliases of
the same kind and allocate bitmap and return it with length.
Anyway if you look at that patches I sent then I call in the driver
of_alias_get_highest_id("serial") which doesn't take care if alias match
with actual driver. It means having information about max alias ID which
match actual driver that would be helpful but I am not quite sure what
should be the flow.
Any link to similar function would be good to understand how the flow is
supposed to work.
Thanks,
Michal
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 1/3] of: base: Introduce of_alias_check_id() to check alias IDs
2018-04-27 6:10 ` Michal Simek
(?)
@ 2018-04-27 13:02 ` Rob Herring
2018-04-27 14:14 ` Michal Simek
-1 siblings, 1 reply; 22+ messages in thread
From: Rob Herring @ 2018-04-27 13:02 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, Michal Simek, One Thousand Gnomes, Alexander Graf,
devicetree, Frank Rowand
On Fri, Apr 27, 2018 at 1:10 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> On 27.4.2018 04:39, Rob Herring wrote:
>> On Thu, Apr 26, 2018 at 9:08 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>> The function travers the lookup table to check if the request alias
>>> id is compatible with the device driver match structure.
>>> This function will be used by serial drivers to check if requested alias
>>> is allocated or free to use.
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> ---
>>>
>>> drivers/of/base.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/of.h | 2 ++
>>> 2 files changed, 51 insertions(+)
>>>
>>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>>> index 848f549164cd..382de01acc72 100644
>>> --- a/drivers/of/base.c
>>> +++ b/drivers/of/base.c
>>> @@ -1892,6 +1892,55 @@ int of_alias_get_id(struct device_node *np, const char *stem)
>>> }
>>> EXPORT_SYMBOL_GPL(of_alias_get_id);
>>>
>>> +/**
>>> + * of_alias_check_id - Check alias id for the give compatibility
>>> + * @matches: Array of of device match structures to search in
>>> + * @stem: Alias stem of the given device_node
>>> + * @id: Alias ID for checking
>>> + *
>>> + * The function travers the lookup table to check if the request alias id
>>> + * is compatible with the device driver match structure
>>> + *
>>> + * Return true if ID is allocated, return false if not
>>> + */
>>> +bool of_alias_check_id(const struct of_device_id *matches, const char *stem,
>>> + int id)
>>
>> Wouldn't it be simpler to just return a bitmap of all allocated ids
>> that match rather than trying to build that up 1 bit at a time?
>
> Is alias list stable or can dt overlay change it?
Stable. If you use an id and then an overlay adds that id as an alias,
what should the kernel do? The only choice is ignore the suggestion.
> What should be the expected flow? Find out maximum number of aliases of
> the same kind and allocate bitmap and return it with length.
I think we can assume <32 so just return a bitmap in a unsigned long
or u32. Use that to initialize a bitmap in the driver. For un-aliased
devices, find the first unset bit, use that id and set the bit.
> Anyway if you look at that patches I sent then I call in the driver
> of_alias_get_highest_id("serial") which doesn't take care if alias match
> with actual driver. It means having information about max alias ID which
> match actual driver that would be helpful but I am not quite sure what
> should be the flow.
I'm a bit confused as to why you use of_alias_get_highest_id and
of_alias_check_id. of_alias_get_highest_id is really intended if you
want to start dynamic allocations above all the alias ids.
I guess the need to match devices is based on each serial driver
having it's own major and /dev name. IMO, we should get rid of those
and every serial driver use ttySn (with the exception of things like
USB serial). Then you'd just need to find all serial aliases. But you
can support both with this function. Just return a bitmap of all
aliases if match is NULL.
> Any link to similar function would be good to understand how the flow is
> supposed to work.
I don't think there's a good model. The pl011 code looks broken to me.
The warning about devices with and without aliases should be fixed.
That is perfectly valid and should be supported. You should really
only need an alias for your console.
Rob
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 1/3] of: base: Introduce of_alias_check_id() to check alias IDs
2018-04-27 13:02 ` Rob Herring
@ 2018-04-27 14:14 ` Michal Simek
0 siblings, 0 replies; 22+ messages in thread
From: Michal Simek @ 2018-04-27 14:14 UTC (permalink / raw)
To: Rob Herring, Michal Simek
Cc: linux-kernel, Michal Simek, One Thousand Gnomes, Alexander Graf,
devicetree, Frank Rowand
On 27.4.2018 15:02, Rob Herring wrote:
> On Fri, Apr 27, 2018 at 1:10 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> On 27.4.2018 04:39, Rob Herring wrote:
>>> On Thu, Apr 26, 2018 at 9:08 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>> The function travers the lookup table to check if the request alias
>>>> id is compatible with the device driver match structure.
>>>> This function will be used by serial drivers to check if requested alias
>>>> is allocated or free to use.
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> ---
>>>>
>>>> drivers/of/base.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
>>>> include/linux/of.h | 2 ++
>>>> 2 files changed, 51 insertions(+)
>>>>
>>>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>>>> index 848f549164cd..382de01acc72 100644
>>>> --- a/drivers/of/base.c
>>>> +++ b/drivers/of/base.c
>>>> @@ -1892,6 +1892,55 @@ int of_alias_get_id(struct device_node *np, const char *stem)
>>>> }
>>>> EXPORT_SYMBOL_GPL(of_alias_get_id);
>>>>
>>>> +/**
>>>> + * of_alias_check_id - Check alias id for the give compatibility
>>>> + * @matches: Array of of device match structures to search in
>>>> + * @stem: Alias stem of the given device_node
>>>> + * @id: Alias ID for checking
>>>> + *
>>>> + * The function travers the lookup table to check if the request alias id
>>>> + * is compatible with the device driver match structure
>>>> + *
>>>> + * Return true if ID is allocated, return false if not
>>>> + */
>>>> +bool of_alias_check_id(const struct of_device_id *matches, const char *stem,
>>>> + int id)
>>>
>>> Wouldn't it be simpler to just return a bitmap of all allocated ids
>>> that match rather than trying to build that up 1 bit at a time?
>>
>> Is alias list stable or can dt overlay change it?
>
> Stable. If you use an id and then an overlay adds that id as an alias,
> what should the kernel do? The only choice is ignore the suggestion.
I didn't play with that for a while but on fpga case you have for
example serial0/serial1 fixed to hard IPs/PS part and then you add 20
uartlites or 16550 IPs to PL via overlays and you want to make sure
proper order for user application.
It means it is not rewriting current but adding new one exactly how you
should do that when fixed and fpga parts are together.
>
>> What should be the expected flow? Find out maximum number of aliases of
>> the same kind and allocate bitmap and return it with length.
>
> I think we can assume <32 so just return a bitmap in a unsigned long
> or u32. Use that to initialize a bitmap in the driver. For un-aliased
> devices, find the first unset bit, use that id and set the bit.
Ok if this is in driver field can be allocated in the driver and passed
also maximum len. It should be better then expecting maximum of 32
devices which ends with max serial31 alias.
Or at least expect u64 with max serial63.
>> Anyway if you look at that patches I sent then I call in the driver
>> of_alias_get_highest_id("serial") which doesn't take care if alias match
>> with actual driver. It means having information about max alias ID which
>> match actual driver that would be helpful but I am not quite sure what
>> should be the flow.
>
> I'm a bit confused as to why you use of_alias_get_highest_id and
> of_alias_check_id. of_alias_get_highest_id is really intended if you
> want to start dynamic allocations above all the alias ids.
I need to get maximum number of ports for passing it to
uart_register_driver which is called only once.
And using of_alias_get_highest_id() return me reasonable number for all
listed serial ports.
It is not correct because some IPs don't need to be listed in that list
but it is reasonable expectation for this RFC.
Even alias bitfield won't help with getting correct number.
The best will be to get number of devices which match that driver
because they don't need to be listed in aliases list.
You get 20 of devices and will allocate space for 20. If there is alias
below 20 you will use numbers. If there is alias >= 20 it will be
ignored in probe and number will be assigned based on empty position.
But this is also no covering overlay cases where others devices can be
added at run time.
> I guess the need to match devices is based on each serial driver
> having it's own major and /dev name. IMO, we should get rid of those
> and every serial driver use ttySn (with the exception of things like
> USB serial). Then you'd just need to find all serial aliases. But you
> can support both with this function. Just return a bitmap of all
> aliases if match is NULL.
This will break a lot of things which are stable today.
>> Any link to similar function would be good to understand how the flow is
>> supposed to work.
>
> I don't think there's a good model. The pl011 code looks broken to me.
yes, it is broken.
> The warning about devices with and without aliases should be fixed.
The same solution which
> That is perfectly valid and should be supported. You should really
> only need an alias for your console.
Having aliases is quite good in case of multiple the same uarts where if
you have more then 2 it is hard to find out which one is the one
connected to certain devices.
With uartlite or ns16550 and PL where you can have unlimited number of
devices this is really must to be able to work with this system.
Thanks,
Michal
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 1/3] of: base: Introduce of_alias_check_id() to check alias IDs
@ 2018-04-27 14:14 ` Michal Simek
0 siblings, 0 replies; 22+ messages in thread
From: Michal Simek @ 2018-04-27 14:14 UTC (permalink / raw)
To: Rob Herring, Michal Simek
Cc: linux-kernel, Michal Simek, One Thousand Gnomes, Alexander Graf,
devicetree, Frank Rowand
On 27.4.2018 15:02, Rob Herring wrote:
> On Fri, Apr 27, 2018 at 1:10 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> On 27.4.2018 04:39, Rob Herring wrote:
>>> On Thu, Apr 26, 2018 at 9:08 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>> The function travers the lookup table to check if the request alias
>>>> id is compatible with the device driver match structure.
>>>> This function will be used by serial drivers to check if requested alias
>>>> is allocated or free to use.
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> ---
>>>>
>>>> drivers/of/base.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
>>>> include/linux/of.h | 2 ++
>>>> 2 files changed, 51 insertions(+)
>>>>
>>>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>>>> index 848f549164cd..382de01acc72 100644
>>>> --- a/drivers/of/base.c
>>>> +++ b/drivers/of/base.c
>>>> @@ -1892,6 +1892,55 @@ int of_alias_get_id(struct device_node *np, const char *stem)
>>>> }
>>>> EXPORT_SYMBOL_GPL(of_alias_get_id);
>>>>
>>>> +/**
>>>> + * of_alias_check_id - Check alias id for the give compatibility
>>>> + * @matches: Array of of device match structures to search in
>>>> + * @stem: Alias stem of the given device_node
>>>> + * @id: Alias ID for checking
>>>> + *
>>>> + * The function travers the lookup table to check if the request alias id
>>>> + * is compatible with the device driver match structure
>>>> + *
>>>> + * Return true if ID is allocated, return false if not
>>>> + */
>>>> +bool of_alias_check_id(const struct of_device_id *matches, const char *stem,
>>>> + int id)
>>>
>>> Wouldn't it be simpler to just return a bitmap of all allocated ids
>>> that match rather than trying to build that up 1 bit at a time?
>>
>> Is alias list stable or can dt overlay change it?
>
> Stable. If you use an id and then an overlay adds that id as an alias,
> what should the kernel do? The only choice is ignore the suggestion.
I didn't play with that for a while but on fpga case you have for
example serial0/serial1 fixed to hard IPs/PS part and then you add 20
uartlites or 16550 IPs to PL via overlays and you want to make sure
proper order for user application.
It means it is not rewriting current but adding new one exactly how you
should do that when fixed and fpga parts are together.
>
>> What should be the expected flow? Find out maximum number of aliases of
>> the same kind and allocate bitmap and return it with length.
>
> I think we can assume <32 so just return a bitmap in a unsigned long
> or u32. Use that to initialize a bitmap in the driver. For un-aliased
> devices, find the first unset bit, use that id and set the bit.
Ok if this is in driver field can be allocated in the driver and passed
also maximum len. It should be better then expecting maximum of 32
devices which ends with max serial31 alias.
Or at least expect u64 with max serial63.
>> Anyway if you look at that patches I sent then I call in the driver
>> of_alias_get_highest_id("serial") which doesn't take care if alias match
>> with actual driver. It means having information about max alias ID which
>> match actual driver that would be helpful but I am not quite sure what
>> should be the flow.
>
> I'm a bit confused as to why you use of_alias_get_highest_id and
> of_alias_check_id. of_alias_get_highest_id is really intended if you
> want to start dynamic allocations above all the alias ids.
I need to get maximum number of ports for passing it to
uart_register_driver which is called only once.
And using of_alias_get_highest_id() return me reasonable number for all
listed serial ports.
It is not correct because some IPs don't need to be listed in that list
but it is reasonable expectation for this RFC.
Even alias bitfield won't help with getting correct number.
The best will be to get number of devices which match that driver
because they don't need to be listed in aliases list.
You get 20 of devices and will allocate space for 20. If there is alias
below 20 you will use numbers. If there is alias >= 20 it will be
ignored in probe and number will be assigned based on empty position.
But this is also no covering overlay cases where others devices can be
added at run time.
> I guess the need to match devices is based on each serial driver
> having it's own major and /dev name. IMO, we should get rid of those
> and every serial driver use ttySn (with the exception of things like
> USB serial). Then you'd just need to find all serial aliases. But you
> can support both with this function. Just return a bitmap of all
> aliases if match is NULL.
This will break a lot of things which are stable today.
>> Any link to similar function would be good to understand how the flow is
>> supposed to work.
>
> I don't think there's a good model. The pl011 code looks broken to me.
yes, it is broken.
> The warning about devices with and without aliases should be fixed.
The same solution which
> That is perfectly valid and should be supported. You should really
> only need an alias for your console.
Having aliases is quite good in case of multiple the same uarts where if
you have more then 2 it is hard to find out which one is the one
connected to certain devices.
With uartlite or ns16550 and PL where you can have unlimited number of
devices this is really must to be able to work with this system.
Thanks,
Michal
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 1/3] of: base: Introduce of_alias_check_id() to check alias IDs
2018-04-27 14:14 ` Michal Simek
(?)
@ 2018-04-27 15:05 ` Rob Herring
-1 siblings, 0 replies; 22+ messages in thread
From: Rob Herring @ 2018-04-27 15:05 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, Michal Simek, One Thousand Gnomes, Alexander Graf,
devicetree, Frank Rowand
On Fri, Apr 27, 2018 at 9:14 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> On 27.4.2018 15:02, Rob Herring wrote:
>> On Fri, Apr 27, 2018 at 1:10 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>> On 27.4.2018 04:39, Rob Herring wrote:
>>>> On Thu, Apr 26, 2018 at 9:08 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>> The function travers the lookup table to check if the request alias
>>>>> id is compatible with the device driver match structure.
>>>>> This function will be used by serial drivers to check if requested alias
>>>>> is allocated or free to use.
>>>>>
>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>> ---
>>>>>
>>>>> drivers/of/base.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>> include/linux/of.h | 2 ++
>>>>> 2 files changed, 51 insertions(+)
>>>>>
>>>>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>>>>> index 848f549164cd..382de01acc72 100644
>>>>> --- a/drivers/of/base.c
>>>>> +++ b/drivers/of/base.c
>>>>> @@ -1892,6 +1892,55 @@ int of_alias_get_id(struct device_node *np, const char *stem)
>>>>> }
>>>>> EXPORT_SYMBOL_GPL(of_alias_get_id);
>>>>>
>>>>> +/**
>>>>> + * of_alias_check_id - Check alias id for the give compatibility
>>>>> + * @matches: Array of of device match structures to search in
>>>>> + * @stem: Alias stem of the given device_node
>>>>> + * @id: Alias ID for checking
>>>>> + *
>>>>> + * The function travers the lookup table to check if the request alias id
>>>>> + * is compatible with the device driver match structure
>>>>> + *
>>>>> + * Return true if ID is allocated, return false if not
>>>>> + */
>>>>> +bool of_alias_check_id(const struct of_device_id *matches, const char *stem,
>>>>> + int id)
>>>>
>>>> Wouldn't it be simpler to just return a bitmap of all allocated ids
>>>> that match rather than trying to build that up 1 bit at a time?
>>>
>>> Is alias list stable or can dt overlay change it?
>>
>> Stable. If you use an id and then an overlay adds that id as an alias,
>> what should the kernel do? The only choice is ignore the suggestion.
>
> I didn't play with that for a while but on fpga case you have for
> example serial0/serial1 fixed to hard IPs/PS part and then you add 20
> uartlites or 16550 IPs to PL via overlays and you want to make sure
> proper order for user application.
> It means it is not rewriting current but adding new one exactly how you
> should do that when fixed and fpga parts are together.
For that to work, it requires knowing the maximum alias number (without alias being present) and allocating dynamic minors above that max alias number.
>>> What should be the expected flow? Find out maximum number of aliases of
>>> the same kind and allocate bitmap and return it with length.
>>
>> I think we can assume <32 so just return a bitmap in a unsigned long
>> or u32. Use that to initialize a bitmap in the driver. For un-aliased
>> devices, find the first unset bit, use that id and set the bit.
>
> Ok if this is in driver field can be allocated in the driver and passed
> also maximum len. It should be better then expecting maximum of 32
> devices which ends with max serial31 alias.
>
> Or at least expect u64 with max serial63.
Sure, 64 should be enough for anyone (TM).
>>> Anyway if you look at that patches I sent then I call in the driver
>>> of_alias_get_highest_id("serial") which doesn't take care if alias match
>>> with actual driver. It means having information about max alias ID which
>>> match actual driver that would be helpful but I am not quite sure what
>>> should be the flow.
>>
>> I'm a bit confused as to why you use of_alias_get_highest_id and
>> of_alias_check_id. of_alias_get_highest_id is really intended if you
>> want to start dynamic allocations above all the alias ids.
>
> I need to get maximum number of ports for passing it to
> uart_register_driver which is called only once.
> And using of_alias_get_highest_id() return me reasonable number for all
> listed serial ports.
> It is not correct because some IPs don't need to be listed in that list
> but it is reasonable expectation for this RFC.
> Even alias bitfield won't help with getting correct number.
>
> The best will be to get number of devices which match that driver
> because they don't need to be listed in aliases list.
of_for_each_compatible()
> You get 20 of devices and will allocate space for 20. If there is alias
> below 20 you will use numbers. If there is alias >= 20 it will be
> ignored in probe and number will be assigned based on empty position.
>
> But this is also no covering overlay cases where others devices can be
> added at run time.
>
>
>> I guess the need to match devices is based on each serial driver
>> having it's own major and /dev name. IMO, we should get rid of those
>> and every serial driver use ttySn (with the exception of things like
>> USB serial). Then you'd just need to find all serial aliases. But you
>> can support both with this function. Just return a bitmap of all
>> aliases if match is NULL.
>
> This will break a lot of things which are stable today.
>
>
>>> Any link to similar function would be good to understand how the flow is
>>> supposed to work.
>>
>> I don't think there's a good model. The pl011 code looks broken to me.
>
> yes, it is broken.
>
>> The warning about devices with and without aliases should be fixed.
>
> The same solution which
>
>> That is perfectly valid and should be supported. You should really
>> only need an alias for your console.
>
> Having aliases is quite good in case of multiple the same uarts where if
> you have more then 2 it is hard to find out which one is the one
> connected to certain devices.
> With uartlite or ns16550 and PL where you can have unlimited number of
> devices this is really must to be able to work with this system.
>
> Thanks,
> Michal
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 1/3] of: base: Introduce of_alias_check_id() to check alias IDs
2018-04-27 14:14 ` Michal Simek
(?)
(?)
@ 2018-04-27 21:58 ` Alexander Graf
2018-04-30 13:44 ` Rob Herring
-1 siblings, 1 reply; 22+ messages in thread
From: Alexander Graf @ 2018-04-27 21:58 UTC (permalink / raw)
To: Michal Simek, Rob Herring
Cc: linux-kernel, Michal Simek, One Thousand Gnomes, devicetree,
Frank Rowand
On 27.04.18 16:14, Michal Simek wrote:
> On 27.4.2018 15:02, Rob Herring wrote:
>> On Fri, Apr 27, 2018 at 1:10 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>> On 27.4.2018 04:39, Rob Herring wrote:
>>>> On Thu, Apr 26, 2018 at 9:08 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>> The function travers the lookup table to check if the request alias
>>>>> id is compatible with the device driver match structure.
>>>>> This function will be used by serial drivers to check if requested alias
>>>>> is allocated or free to use.
>>>>>
>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>> ---
>>>>>
>>>>> drivers/of/base.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>> include/linux/of.h | 2 ++
>>>>> 2 files changed, 51 insertions(+)
>>>>>
>>>>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>>>>> index 848f549164cd..382de01acc72 100644
>>>>> --- a/drivers/of/base.c
>>>>> +++ b/drivers/of/base.c
>>>>> @@ -1892,6 +1892,55 @@ int of_alias_get_id(struct device_node *np, const char *stem)
>>>>> }
>>>>> EXPORT_SYMBOL_GPL(of_alias_get_id);
>>>>>
>>>>> +/**
>>>>> + * of_alias_check_id - Check alias id for the give compatibility
>>>>> + * @matches: Array of of device match structures to search in
>>>>> + * @stem: Alias stem of the given device_node
>>>>> + * @id: Alias ID for checking
>>>>> + *
>>>>> + * The function travers the lookup table to check if the request alias id
>>>>> + * is compatible with the device driver match structure
>>>>> + *
>>>>> + * Return true if ID is allocated, return false if not
>>>>> + */
>>>>> +bool of_alias_check_id(const struct of_device_id *matches, const char *stem,
>>>>> + int id)
>>>>
>>>> Wouldn't it be simpler to just return a bitmap of all allocated ids
>>>> that match rather than trying to build that up 1 bit at a time?
>>>
>>> Is alias list stable or can dt overlay change it?
>>
>> Stable. If you use an id and then an overlay adds that id as an alias,
>> what should the kernel do? The only choice is ignore the suggestion.
>
> I didn't play with that for a while but on fpga case you have for
> example serial0/serial1 fixed to hard IPs/PS part and then you add 20
> uartlites or 16550 IPs to PL via overlays and you want to make sure
> proper order for user application.
> It means it is not rewriting current but adding new one exactly how you
> should do that when fixed and fpga parts are together.
>
>
>>
>>> What should be the expected flow? Find out maximum number of aliases of
>>> the same kind and allocate bitmap and return it with length.
>>
>> I think we can assume <32 so just return a bitmap in a unsigned long
>> or u32. Use that to initialize a bitmap in the driver. For un-aliased
>> devices, find the first unset bit, use that id and set the bit.
>
> Ok if this is in driver field can be allocated in the driver and passed
> also maximum len. It should be better then expecting maximum of 32
> devices which ends with max serial31 alias.
>
> Or at least expect u64 with max serial63.
>
>
>>> Anyway if you look at that patches I sent then I call in the driver
>>> of_alias_get_highest_id("serial") which doesn't take care if alias match
>>> with actual driver. It means having information about max alias ID which
>>> match actual driver that would be helpful but I am not quite sure what
>>> should be the flow.
>>
>> I'm a bit confused as to why you use of_alias_get_highest_id and
>> of_alias_check_id. of_alias_get_highest_id is really intended if you
>> want to start dynamic allocations above all the alias ids.
>
> I need to get maximum number of ports for passing it to
> uart_register_driver which is called only once.
> And using of_alias_get_highest_id() return me reasonable number for all
> listed serial ports.
> It is not correct because some IPs don't need to be listed in that list
> but it is reasonable expectation for this RFC.
> Even alias bitfield won't help with getting correct number.
>
>
> The best will be to get number of devices which match that driver
> because they don't need to be listed in aliases list.
>
> You get 20 of devices and will allocate space for 20. If there is alias
> below 20 you will use numbers. If there is alias >= 20 it will be
> ignored in probe and number will be assigned based on empty position.
>
> But this is also no covering overlay cases where others devices can be
> added at run time.
>
>
>> I guess the need to match devices is based on each serial driver
>> having it's own major and /dev name. IMO, we should get rid of those
>> and every serial driver use ttySn (with the exception of things like
>> USB serial). Then you'd just need to find all serial aliases. But you
>> can support both with this function. Just return a bitmap of all
>> aliases if match is NULL.
>
> This will break a lot of things which are stable today.
What if ttySn were simply symlinks and no driver spawns any device of
that name? So the normal NS16550 UART driver would get a new name, but
ttyS0 we could still match for everywhere regardless.
In that layer we could then do the alias mapping and simply honor the
numbers given by DT.
Alex
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 1/3] of: base: Introduce of_alias_check_id() to check alias IDs
2018-04-27 21:58 ` Alexander Graf
@ 2018-04-30 13:44 ` Rob Herring
0 siblings, 0 replies; 22+ messages in thread
From: Rob Herring @ 2018-04-30 13:44 UTC (permalink / raw)
To: Alexander Graf
Cc: Michal Simek, linux-kernel, Michal Simek, One Thousand Gnomes,
devicetree, Frank Rowand
On Fri, Apr 27, 2018 at 4:58 PM, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 27.04.18 16:14, Michal Simek wrote:
>> On 27.4.2018 15:02, Rob Herring wrote:
>>> On Fri, Apr 27, 2018 at 1:10 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>> On 27.4.2018 04:39, Rob Herring wrote:
>>>>> On Thu, Apr 26, 2018 at 9:08 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>> The function travers the lookup table to check if the request alias
>>>>>> id is compatible with the device driver match structure.
>>>>>> This function will be used by serial drivers to check if requested alias
>>>>>> is allocated or free to use.
>>>>>>
>>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>>> ---
>>>>>>
>>>>>> drivers/of/base.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>>> include/linux/of.h | 2 ++
>>>>>> 2 files changed, 51 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>>>>>> index 848f549164cd..382de01acc72 100644
>>>>>> --- a/drivers/of/base.c
>>>>>> +++ b/drivers/of/base.c
>>>>>> @@ -1892,6 +1892,55 @@ int of_alias_get_id(struct device_node *np, const char *stem)
>>>>>> }
>>>>>> EXPORT_SYMBOL_GPL(of_alias_get_id);
>>>>>>
>>>>>> +/**
>>>>>> + * of_alias_check_id - Check alias id for the give compatibility
>>>>>> + * @matches: Array of of device match structures to search in
>>>>>> + * @stem: Alias stem of the given device_node
>>>>>> + * @id: Alias ID for checking
>>>>>> + *
>>>>>> + * The function travers the lookup table to check if the request alias id
>>>>>> + * is compatible with the device driver match structure
>>>>>> + *
>>>>>> + * Return true if ID is allocated, return false if not
>>>>>> + */
>>>>>> +bool of_alias_check_id(const struct of_device_id *matches, const char *stem,
>>>>>> + int id)
>>>>>
>>>>> Wouldn't it be simpler to just return a bitmap of all allocated ids
>>>>> that match rather than trying to build that up 1 bit at a time?
>>>>
>>>> Is alias list stable or can dt overlay change it?
>>>
>>> Stable. If you use an id and then an overlay adds that id as an alias,
>>> what should the kernel do? The only choice is ignore the suggestion.
>>
>> I didn't play with that for a while but on fpga case you have for
>> example serial0/serial1 fixed to hard IPs/PS part and then you add 20
>> uartlites or 16550 IPs to PL via overlays and you want to make sure
>> proper order for user application.
>> It means it is not rewriting current but adding new one exactly how you
>> should do that when fixed and fpga parts are together.
>>
>>
>>>
>>>> What should be the expected flow? Find out maximum number of aliases of
>>>> the same kind and allocate bitmap and return it with length.
>>>
>>> I think we can assume <32 so just return a bitmap in a unsigned long
>>> or u32. Use that to initialize a bitmap in the driver. For un-aliased
>>> devices, find the first unset bit, use that id and set the bit.
>>
>> Ok if this is in driver field can be allocated in the driver and passed
>> also maximum len. It should be better then expecting maximum of 32
>> devices which ends with max serial31 alias.
>>
>> Or at least expect u64 with max serial63.
>>
>>
>>>> Anyway if you look at that patches I sent then I call in the driver
>>>> of_alias_get_highest_id("serial") which doesn't take care if alias match
>>>> with actual driver. It means having information about max alias ID which
>>>> match actual driver that would be helpful but I am not quite sure what
>>>> should be the flow.
>>>
>>> I'm a bit confused as to why you use of_alias_get_highest_id and
>>> of_alias_check_id. of_alias_get_highest_id is really intended if you
>>> want to start dynamic allocations above all the alias ids.
>>
>> I need to get maximum number of ports for passing it to
>> uart_register_driver which is called only once.
>> And using of_alias_get_highest_id() return me reasonable number for all
>> listed serial ports.
>> It is not correct because some IPs don't need to be listed in that list
>> but it is reasonable expectation for this RFC.
>> Even alias bitfield won't help with getting correct number.
>>
>>
>> The best will be to get number of devices which match that driver
>> because they don't need to be listed in aliases list.
>>
>> You get 20 of devices and will allocate space for 20. If there is alias
>> below 20 you will use numbers. If there is alias >= 20 it will be
>> ignored in probe and number will be assigned based on empty position.
>>
>> But this is also no covering overlay cases where others devices can be
>> added at run time.
>>
>>
>>> I guess the need to match devices is based on each serial driver
>>> having it's own major and /dev name. IMO, we should get rid of those
>>> and every serial driver use ttySn (with the exception of things like
>>> USB serial). Then you'd just need to find all serial aliases. But you
>>> can support both with this function. Just return a bitmap of all
>>> aliases if match is NULL.
>>
>> This will break a lot of things which are stable today.
>
> What if ttySn were simply symlinks and no driver spawns any device of
> that name? So the normal NS16550 UART driver would get a new name, but
> ttyS0 we could still match for everywhere regardless.
I'd always figured we'd make the old names symlinks to the new names.
> In that layer we could then do the alias mapping and simply honor the
> numbers given by DT.
Is that really necessary?
>
>
> Alex
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH 2/3] serial: uartps: Move register to probe based on run time detection
2018-04-26 14:08 ` Michal Simek
@ 2018-04-26 14:08 ` Michal Simek
-1 siblings, 0 replies; 22+ messages in thread
From: Michal Simek @ 2018-04-26 14:08 UTC (permalink / raw)
To: linux-kernel, monstr, gnomes, Alexander Graf
Cc: Jiri Slaby, linux-serial, Greg Kroah-Hartman, linux-arm-kernel
Find out the highest serial alias and allocate that amount of
structures/minor numbers to be able to handle all of them.
Origin setting that there are two prealocated CDNS_UART_NR_PORTS is kept
there.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Discussed here: https://patchwork.kernel.org/patch/9738445/
The same solution is done in pl011 driver.
---
drivers/tty/serial/xilinx_uartps.c | 32 ++++++++++++++++--------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 0fc9de20843c..ffb5b66a05b5 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1432,6 +1432,22 @@ static int cdns_uart_probe(struct platform_device *pdev)
struct cdns_uart *cdns_uart_data;
const struct of_device_id *match;
+ if (!cdns_uart_uart_driver.state) {
+ rc = of_alias_get_highest_id("serial");
+ if (rc >= CDNS_UART_NR_PORTS) {
+ /* serial2 means 3 ports that's why rc(2) + 1 here */
+ rc += 1;
+ /* Rewrite default setup which is using 2 ports */
+ cdns_uart_uart_driver.nr = rc;
+ }
+
+ rc = uart_register_driver(&cdns_uart_uart_driver);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "Failed to register driver\n");
+ return rc;
+ }
+ }
+
cdns_uart_data = devm_kzalloc(&pdev->dev, sizeof(*cdns_uart_data),
GFP_KERNEL);
if (!cdns_uart_data)
@@ -1622,28 +1638,14 @@ static struct platform_driver cdns_uart_platform_driver = {
static int __init cdns_uart_init(void)
{
- int retval = 0;
-
- /* Register the cdns_uart driver with the serial core */
- retval = uart_register_driver(&cdns_uart_uart_driver);
- if (retval)
- return retval;
-
/* Register the platform driver */
- retval = platform_driver_register(&cdns_uart_platform_driver);
- if (retval)
- uart_unregister_driver(&cdns_uart_uart_driver);
-
- return retval;
+ return platform_driver_register(&cdns_uart_platform_driver);
}
static void __exit cdns_uart_exit(void)
{
/* Unregister the platform driver */
platform_driver_unregister(&cdns_uart_platform_driver);
-
- /* Unregister the cdns_uart driver */
- uart_unregister_driver(&cdns_uart_uart_driver);
}
arch_initcall(cdns_uart_init);
--
2.17.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 2/3] serial: uartps: Move register to probe based on run time detection
@ 2018-04-26 14:08 ` Michal Simek
0 siblings, 0 replies; 22+ messages in thread
From: Michal Simek @ 2018-04-26 14:08 UTC (permalink / raw)
To: linux-arm-kernel
Find out the highest serial alias and allocate that amount of
structures/minor numbers to be able to handle all of them.
Origin setting that there are two prealocated CDNS_UART_NR_PORTS is kept
there.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Discussed here: https://patchwork.kernel.org/patch/9738445/
The same solution is done in pl011 driver.
---
drivers/tty/serial/xilinx_uartps.c | 32 ++++++++++++++++--------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 0fc9de20843c..ffb5b66a05b5 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1432,6 +1432,22 @@ static int cdns_uart_probe(struct platform_device *pdev)
struct cdns_uart *cdns_uart_data;
const struct of_device_id *match;
+ if (!cdns_uart_uart_driver.state) {
+ rc = of_alias_get_highest_id("serial");
+ if (rc >= CDNS_UART_NR_PORTS) {
+ /* serial2 means 3 ports that's why rc(2) + 1 here */
+ rc += 1;
+ /* Rewrite default setup which is using 2 ports */
+ cdns_uart_uart_driver.nr = rc;
+ }
+
+ rc = uart_register_driver(&cdns_uart_uart_driver);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "Failed to register driver\n");
+ return rc;
+ }
+ }
+
cdns_uart_data = devm_kzalloc(&pdev->dev, sizeof(*cdns_uart_data),
GFP_KERNEL);
if (!cdns_uart_data)
@@ -1622,28 +1638,14 @@ static struct platform_driver cdns_uart_platform_driver = {
static int __init cdns_uart_init(void)
{
- int retval = 0;
-
- /* Register the cdns_uart driver with the serial core */
- retval = uart_register_driver(&cdns_uart_uart_driver);
- if (retval)
- return retval;
-
/* Register the platform driver */
- retval = platform_driver_register(&cdns_uart_platform_driver);
- if (retval)
- uart_unregister_driver(&cdns_uart_uart_driver);
-
- return retval;
+ return platform_driver_register(&cdns_uart_platform_driver);
}
static void __exit cdns_uart_exit(void)
{
/* Unregister the platform driver */
platform_driver_unregister(&cdns_uart_platform_driver);
-
- /* Unregister the cdns_uart driver */
- uart_unregister_driver(&cdns_uart_uart_driver);
}
arch_initcall(cdns_uart_init);
--
2.17.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [RFC PATCH 3/3] serial: uartps: Change uart ports allocation
2018-04-26 14:08 ` Michal Simek
@ 2018-04-26 14:08 ` Michal Simek
-1 siblings, 0 replies; 22+ messages in thread
From: Michal Simek @ 2018-04-26 14:08 UTC (permalink / raw)
To: linux-kernel, monstr, gnomes, Alexander Graf
Cc: Jiri Slaby, linux-serial, Greg Kroah-Hartman, linux-arm-kernel
For ips which have alias algorightm all the tim using that alias and
minor number. It means serial20 alias ends up as ttyPS20.
If aliases are not setup for enabled core the first unused position is
used but that's need to be checked if it is really empty because another
instance doesn't need to be probed. of_alias_check_id() does that test.
If alias pointing to different not compatible IP, it is free to use.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/tty/serial/xilinx_uartps.c | 96 ++++++++++++++++++++++++++----
1 file changed, 85 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index ffb5b66a05b5..7c616b48c117 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -45,6 +45,9 @@ static int rx_timeout = 10;
module_param(rx_timeout, uint, S_IRUGO);
MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-255");
+/* The inuse bitfield is recording which IDs are free/used by the driver. */
+static unsigned long *inuse;
+
/* Register offsets for the UART. */
#define CDNS_UART_CR 0x00 /* Control Register */
#define CDNS_UART_MR 0x04 /* Mode Register */
@@ -1418,6 +1421,75 @@ static const struct of_device_id cdns_uart_of_match[] = {
};
MODULE_DEVICE_TABLE(of, cdns_uart_of_match);
+static int cdns_get_id(struct platform_device *pdev)
+{
+ int id;
+
+ /* Look for a serialN alias */
+ id = of_alias_get_id(pdev->dev.of_node, "serial");
+ if (id < 0) {
+ dev_warn(&pdev->dev,
+ "No serial alias passed. Using the first free id\n");
+
+ /*
+ * Start with id 0 and check if there is no serial0 alias
+ * which points to device which is compatible with this driver.
+ * If alias exists then try next free position.
+ */
+ id = 0;
+
+ for (;;) {
+ dev_dbg(&pdev->dev, "Validate id %d\n", id);
+ id = find_next_zero_bit(inuse, id,
+ cdns_uart_uart_driver.nr);
+
+ dev_dbg(&pdev->dev, "The empty id is %d\n", id);
+ /* Check if ID is empty */
+
+ if (of_alias_check_id(cdns_uart_of_match,
+ "serial", id)) {
+ dev_dbg(&pdev->dev,
+ "ID %d already taken - skipped\n", id);
+ /* Try next one */
+ id++;
+ continue;
+ }
+
+ /* It shouldn't happen anytime */
+ if (id >= cdns_uart_uart_driver.nr) {
+ dev_dbg(&pdev->dev,
+ "ID %d is greater than NR of ports\n",
+ cdns_uart_uart_driver.nr);
+ return -ENXIO;
+ }
+
+ /* Break the loop if bit is taken */
+ if (!test_and_set_bit(id, inuse)) {
+ dev_dbg(&pdev->dev,
+ "Selected ID %d allocation passed\n",
+ id);
+ break;
+ }
+ dev_dbg(&pdev->dev,
+ "Selected ID %d allocation failed\n", id);
+ /* if taking bit fails then try next one */
+ id++;
+ }
+ } else {
+ /*
+ * If alias was found there is no reason to check
+ * anything else
+ */
+ if (test_and_set_bit(id, inuse)) {
+ dev_dbg(&pdev->dev,
+ "ID %d allocation failed\n", id);
+ return -EAGAIN;
+ }
+ }
+
+ return id;
+}
+
/**
* cdns_uart_probe - Platform driver probe
* @pdev: Pointer to the platform device structure
@@ -1446,6 +1518,13 @@ static int cdns_uart_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "Failed to register driver\n");
return rc;
}
+
+ /* Create bitfield to record which ids are free */
+ inuse = devm_kcalloc(&pdev->dev,
+ BITS_TO_LONGS(cdns_uart_uart_driver.nr),
+ sizeof(unsigned long), GFP_KERNEL);
+ if (!inuse)
+ return -ENOMEM;
}
cdns_uart_data = devm_kzalloc(&pdev->dev, sizeof(*cdns_uart_data),
@@ -1463,6 +1542,10 @@ static int cdns_uart_probe(struct platform_device *pdev)
cdns_uart_data->quirks = data->quirks;
}
+ id = cdns_get_id(pdev);
+ if (id < 0)
+ return id;
+
cdns_uart_data->pclk = devm_clk_get(&pdev->dev, "pclk");
if (IS_ERR(cdns_uart_data->pclk)) {
cdns_uart_data->pclk = devm_clk_get(&pdev->dev, "aper_clk");
@@ -1515,16 +1598,6 @@ static int cdns_uart_probe(struct platform_device *pdev)
&cdns_uart_data->clk_rate_change_nb))
dev_warn(&pdev->dev, "Unable to register clock notifier.\n");
#endif
- /* Look for a serialN alias */
- id = of_alias_get_id(pdev->dev.of_node, "serial");
- if (id < 0)
- id = 0;
-
- if (id >= CDNS_UART_NR_PORTS) {
- dev_err(&pdev->dev, "Cannot get uart_port structure\n");
- rc = -ENODEV;
- goto err_out_notif_unreg;
- }
/* At this point, we've got an empty uart_port struct, initialize it */
spin_lock_init(&port->lock);
@@ -1583,10 +1656,10 @@ static int cdns_uart_probe(struct platform_device *pdev)
return 0;
err_out_pm_disable:
+ clear_bit(port->line, inuse);
pm_runtime_disable(&pdev->dev);
pm_runtime_set_suspended(&pdev->dev);
pm_runtime_dont_use_autosuspend(&pdev->dev);
-err_out_notif_unreg:
#ifdef CONFIG_COMMON_CLK
clk_notifier_unregister(cdns_uart_data->uartclk,
&cdns_uart_data->clk_rate_change_nb);
@@ -1618,6 +1691,7 @@ static int cdns_uart_remove(struct platform_device *pdev)
#endif
rc = uart_remove_one_port(&cdns_uart_uart_driver, port);
port->mapbase = 0;
+ clear_bit(port->line, inuse);
clk_disable_unprepare(cdns_uart_data->uartclk);
clk_disable_unprepare(cdns_uart_data->pclk);
pm_runtime_disable(&pdev->dev);
--
2.17.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH 3/3] serial: uartps: Change uart ports allocation
@ 2018-04-26 14:08 ` Michal Simek
0 siblings, 0 replies; 22+ messages in thread
From: Michal Simek @ 2018-04-26 14:08 UTC (permalink / raw)
To: linux-arm-kernel
For ips which have alias algorightm all the tim using that alias and
minor number. It means serial20 alias ends up as ttyPS20.
If aliases are not setup for enabled core the first unused position is
used but that's need to be checked if it is really empty because another
instance doesn't need to be probed. of_alias_check_id() does that test.
If alias pointing to different not compatible IP, it is free to use.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/tty/serial/xilinx_uartps.c | 96 ++++++++++++++++++++++++++----
1 file changed, 85 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index ffb5b66a05b5..7c616b48c117 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -45,6 +45,9 @@ static int rx_timeout = 10;
module_param(rx_timeout, uint, S_IRUGO);
MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-255");
+/* The inuse bitfield is recording which IDs are free/used by the driver. */
+static unsigned long *inuse;
+
/* Register offsets for the UART. */
#define CDNS_UART_CR 0x00 /* Control Register */
#define CDNS_UART_MR 0x04 /* Mode Register */
@@ -1418,6 +1421,75 @@ static const struct of_device_id cdns_uart_of_match[] = {
};
MODULE_DEVICE_TABLE(of, cdns_uart_of_match);
+static int cdns_get_id(struct platform_device *pdev)
+{
+ int id;
+
+ /* Look for a serialN alias */
+ id = of_alias_get_id(pdev->dev.of_node, "serial");
+ if (id < 0) {
+ dev_warn(&pdev->dev,
+ "No serial alias passed. Using the first free id\n");
+
+ /*
+ * Start with id 0 and check if there is no serial0 alias
+ * which points to device which is compatible with this driver.
+ * If alias exists then try next free position.
+ */
+ id = 0;
+
+ for (;;) {
+ dev_dbg(&pdev->dev, "Validate id %d\n", id);
+ id = find_next_zero_bit(inuse, id,
+ cdns_uart_uart_driver.nr);
+
+ dev_dbg(&pdev->dev, "The empty id is %d\n", id);
+ /* Check if ID is empty */
+
+ if (of_alias_check_id(cdns_uart_of_match,
+ "serial", id)) {
+ dev_dbg(&pdev->dev,
+ "ID %d already taken - skipped\n", id);
+ /* Try next one */
+ id++;
+ continue;
+ }
+
+ /* It shouldn't happen anytime */
+ if (id >= cdns_uart_uart_driver.nr) {
+ dev_dbg(&pdev->dev,
+ "ID %d is greater than NR of ports\n",
+ cdns_uart_uart_driver.nr);
+ return -ENXIO;
+ }
+
+ /* Break the loop if bit is taken */
+ if (!test_and_set_bit(id, inuse)) {
+ dev_dbg(&pdev->dev,
+ "Selected ID %d allocation passed\n",
+ id);
+ break;
+ }
+ dev_dbg(&pdev->dev,
+ "Selected ID %d allocation failed\n", id);
+ /* if taking bit fails then try next one */
+ id++;
+ }
+ } else {
+ /*
+ * If alias was found there is no reason to check
+ * anything else
+ */
+ if (test_and_set_bit(id, inuse)) {
+ dev_dbg(&pdev->dev,
+ "ID %d allocation failed\n", id);
+ return -EAGAIN;
+ }
+ }
+
+ return id;
+}
+
/**
* cdns_uart_probe - Platform driver probe
* @pdev: Pointer to the platform device structure
@@ -1446,6 +1518,13 @@ static int cdns_uart_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "Failed to register driver\n");
return rc;
}
+
+ /* Create bitfield to record which ids are free */
+ inuse = devm_kcalloc(&pdev->dev,
+ BITS_TO_LONGS(cdns_uart_uart_driver.nr),
+ sizeof(unsigned long), GFP_KERNEL);
+ if (!inuse)
+ return -ENOMEM;
}
cdns_uart_data = devm_kzalloc(&pdev->dev, sizeof(*cdns_uart_data),
@@ -1463,6 +1542,10 @@ static int cdns_uart_probe(struct platform_device *pdev)
cdns_uart_data->quirks = data->quirks;
}
+ id = cdns_get_id(pdev);
+ if (id < 0)
+ return id;
+
cdns_uart_data->pclk = devm_clk_get(&pdev->dev, "pclk");
if (IS_ERR(cdns_uart_data->pclk)) {
cdns_uart_data->pclk = devm_clk_get(&pdev->dev, "aper_clk");
@@ -1515,16 +1598,6 @@ static int cdns_uart_probe(struct platform_device *pdev)
&cdns_uart_data->clk_rate_change_nb))
dev_warn(&pdev->dev, "Unable to register clock notifier.\n");
#endif
- /* Look for a serialN alias */
- id = of_alias_get_id(pdev->dev.of_node, "serial");
- if (id < 0)
- id = 0;
-
- if (id >= CDNS_UART_NR_PORTS) {
- dev_err(&pdev->dev, "Cannot get uart_port structure\n");
- rc = -ENODEV;
- goto err_out_notif_unreg;
- }
/* At this point, we've got an empty uart_port struct, initialize it */
spin_lock_init(&port->lock);
@@ -1583,10 +1656,10 @@ static int cdns_uart_probe(struct platform_device *pdev)
return 0;
err_out_pm_disable:
+ clear_bit(port->line, inuse);
pm_runtime_disable(&pdev->dev);
pm_runtime_set_suspended(&pdev->dev);
pm_runtime_dont_use_autosuspend(&pdev->dev);
-err_out_notif_unreg:
#ifdef CONFIG_COMMON_CLK
clk_notifier_unregister(cdns_uart_data->uartclk,
&cdns_uart_data->clk_rate_change_nb);
@@ -1618,6 +1691,7 @@ static int cdns_uart_remove(struct platform_device *pdev)
#endif
rc = uart_remove_one_port(&cdns_uart_uart_driver, port);
port->mapbase = 0;
+ clear_bit(port->line, inuse);
clk_disable_unprepare(cdns_uart_data->uartclk);
clk_disable_unprepare(cdns_uart_data->pclk);
pm_runtime_disable(&pdev->dev);
--
2.17.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 0/3] serial: uartps: Add run time support for more IPs than hardcoded 2
2018-04-26 14:08 ` Michal Simek
(?)
@ 2018-05-05 13:10 ` Maarten Brock
-1 siblings, 0 replies; 22+ messages in thread
From: Maarten Brock @ 2018-05-05 13:10 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, monstr, gnomes, Alexander Graf, devicetree,
Jiri Slaby, Greg Kroah-Hartman, Rob Herring, linux-serial,
Frank Rowand, linux-arm-kernel, linux-serial-owner
On 2018-04-26 16:08, Michal Simek wrote:
> Hi,
>
> this series is trying to address discussion I had with Alan in past
> https://patchwork.kernel.org/patch/9738445/.
>
> It is moving uart_register_driver() to probe function like it is done
> in
> pl011 driver.
> And also introducing new function for alias compatibility checking to
> resolve cases where some IPs have alias and some of them not.
> This case is detected in pl011_probe_dt_alias() but not properly
> solved.
>
> Also keep status of free ids/minor numbers in bitmap to know what's the
> next unallocated number.
>
> The same solution can be used in pl011, uart16550 and uartlite to
> really
> get unified kernel.
>
> Tested on these use cases:
> Notes:
> ff000000 is the first PS uart listed in dtsi
> ff010000 is the second PS uart listed in dtsi
>
> Standard zcu102 setting
> serial0 = &uart0;
> serial1 = &uart1;
> [ 0.196628] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.642542] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> Swapped zcu102 setting
> serial0 = &uart1;
> serial1 = &uart0;
> [ 0.196472] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.196824] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> uart0 on alias higher then MAX uart ports
> serial0 = &uart1;
> serial200 = &uart0;
> [ 0.176793] ff000000.serial: ttyPS200 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.177288] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> Both uarts on higher aliases
> serial1 = &uart1;
> serial2 = &uart0;
> [ 0.196470] ff000000.serial: ttyPS2 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.196823] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> uart0 not listed but it is probed first that's why should be ttyPS0 but
> there is uart1 via alias
> serial0 = &uart1;
> [ 0.176656] xuartps ff000000.serial: No serial alias passed. Using
> the first free id
> [ 0.176671] xuartps ff000000.serial: Validate id 0
> [ 0.176680] xuartps ff000000.serial: The empty id is 0
> [ 0.176692] xuartps ff000000.serial: ID 0 already taken - skipped
> [ 0.176701] xuartps ff000000.serial: Validate id 1
> [ 0.176710] xuartps ff000000.serial: The empty id is 1
> [ 0.176719] xuartps ff000000.serial: Selected ID 1 allocation passed
> [ 0.176760] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.177104] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> uart0 not listed but it is probed first that's why should be ttyPS0
> serial1 = &uart1;
> [ 0.176661] xuartps ff000000.serial: No serial alias passed. Using
> the first free id
> [ 0.176676] xuartps ff000000.serial: Validate id 0
> [ 0.176686] xuartps ff000000.serial: The empty id is 0
> [ 0.176696] xuartps ff000000.serial: Selected ID 0 allocation passed
> [ 0.176737] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.177069] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> uarts not listed in aliases list
> [ 0.176673] xuartps ff000000.serial: No serial alias passed. Using
> the first free id
> [ 0.176687] xuartps ff000000.serial: Validate id 0
> [ 0.176697] xuartps ff000000.serial: The empty id is 0
> [ 0.176707] xuartps ff000000.serial: Selected ID 0 allocation passed
> [ 0.176746] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.177057] xuartps ff010000.serial: No serial alias passed. Using
> the first free id
> [ 0.177070] xuartps ff010000.serial: Validate id 0
> [ 0.177079] xuartps ff010000.serial: The empty id is 0
> [ 0.177089] xuartps ff010000.serial: Selected ID 0 allocation failed
> [ 0.177098] xuartps ff010000.serial: Validate id 1
> [ 0.177107] xuartps ff010000.serial: The empty id is 1
> [ 0.177116] xuartps ff010000.serial: Selected ID 1 allocation passed
> [ 0.177149] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> Thanks,
> Michal
Hello Michal,
How will this interact with ns16550 based UARTs?
Can we have both /dev/ttyS0 and /dev/ttyPS0?
Currently we can't unless we create *no* serialN alias for the ns16550.
And there is no other means to lock the /dev/ttySx name to a device
either.
Or will the xuartps driver eventually use /dev/ttySx as well?
Maarten
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH 0/3] serial: uartps: Add run time support for more IPs than hardcoded 2
@ 2018-05-05 13:10 ` Maarten Brock
0 siblings, 0 replies; 22+ messages in thread
From: Maarten Brock @ 2018-05-05 13:10 UTC (permalink / raw)
To: linux-arm-kernel
On 2018-04-26 16:08, Michal Simek wrote:
> Hi,
>
> this series is trying to address discussion I had with Alan in past
> https://patchwork.kernel.org/patch/9738445/.
>
> It is moving uart_register_driver() to probe function like it is done
> in
> pl011 driver.
> And also introducing new function for alias compatibility checking to
> resolve cases where some IPs have alias and some of them not.
> This case is detected in pl011_probe_dt_alias() but not properly
> solved.
>
> Also keep status of free ids/minor numbers in bitmap to know what's the
> next unallocated number.
>
> The same solution can be used in pl011, uart16550 and uartlite to
> really
> get unified kernel.
>
> Tested on these use cases:
> Notes:
> ff000000 is the first PS uart listed in dtsi
> ff010000 is the second PS uart listed in dtsi
>
> Standard zcu102 setting
> serial0 = &uart0;
> serial1 = &uart1;
> [ 0.196628] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.642542] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> Swapped zcu102 setting
> serial0 = &uart1;
> serial1 = &uart0;
> [ 0.196472] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.196824] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> uart0 on alias higher then MAX uart ports
> serial0 = &uart1;
> serial200 = &uart0;
> [ 0.176793] ff000000.serial: ttyPS200 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.177288] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> Both uarts on higher aliases
> serial1 = &uart1;
> serial2 = &uart0;
> [ 0.196470] ff000000.serial: ttyPS2 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.196823] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> uart0 not listed but it is probed first that's why should be ttyPS0 but
> there is uart1 via alias
> serial0 = &uart1;
> [ 0.176656] xuartps ff000000.serial: No serial alias passed. Using
> the first free id
> [ 0.176671] xuartps ff000000.serial: Validate id 0
> [ 0.176680] xuartps ff000000.serial: The empty id is 0
> [ 0.176692] xuartps ff000000.serial: ID 0 already taken - skipped
> [ 0.176701] xuartps ff000000.serial: Validate id 1
> [ 0.176710] xuartps ff000000.serial: The empty id is 1
> [ 0.176719] xuartps ff000000.serial: Selected ID 1 allocation passed
> [ 0.176760] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.177104] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> uart0 not listed but it is probed first that's why should be ttyPS0
> serial1 = &uart1;
> [ 0.176661] xuartps ff000000.serial: No serial alias passed. Using
> the first free id
> [ 0.176676] xuartps ff000000.serial: Validate id 0
> [ 0.176686] xuartps ff000000.serial: The empty id is 0
> [ 0.176696] xuartps ff000000.serial: Selected ID 0 allocation passed
> [ 0.176737] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.177069] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> uarts not listed in aliases list
> [ 0.176673] xuartps ff000000.serial: No serial alias passed. Using
> the first free id
> [ 0.176687] xuartps ff000000.serial: Validate id 0
> [ 0.176697] xuartps ff000000.serial: The empty id is 0
> [ 0.176707] xuartps ff000000.serial: Selected ID 0 allocation passed
> [ 0.176746] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.177057] xuartps ff010000.serial: No serial alias passed. Using
> the first free id
> [ 0.177070] xuartps ff010000.serial: Validate id 0
> [ 0.177079] xuartps ff010000.serial: The empty id is 0
> [ 0.177089] xuartps ff010000.serial: Selected ID 0 allocation failed
> [ 0.177098] xuartps ff010000.serial: Validate id 1
> [ 0.177107] xuartps ff010000.serial: The empty id is 1
> [ 0.177116] xuartps ff010000.serial: Selected ID 1 allocation passed
> [ 0.177149] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> Thanks,
> Michal
Hello Michal,
How will this interact with ns16550 based UARTs?
Can we have both /dev/ttyS0 and /dev/ttyPS0?
Currently we can't unless we create *no* serialN alias for the ns16550.
And there is no other means to lock the /dev/ttySx name to a device
either.
Or will the xuartps driver eventually use /dev/ttySx as well?
Maarten
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 0/3] serial: uartps: Add run time support for more IPs than hardcoded 2
@ 2018-05-05 13:10 ` Maarten Brock
0 siblings, 0 replies; 22+ messages in thread
From: Maarten Brock @ 2018-05-05 13:10 UTC (permalink / raw)
To: Michal Simek
Cc: gnomes, monstr, linux-serial-owner, devicetree,
Greg Kroah-Hartman, linux-kernel, Alexander Graf, Rob Herring,
linux-serial, Jiri Slaby, Frank Rowand, linux-arm-kernel
On 2018-04-26 16:08, Michal Simek wrote:
> Hi,
>
> this series is trying to address discussion I had with Alan in past
> https://patchwork.kernel.org/patch/9738445/.
>
> It is moving uart_register_driver() to probe function like it is done
> in
> pl011 driver.
> And also introducing new function for alias compatibility checking to
> resolve cases where some IPs have alias and some of them not.
> This case is detected in pl011_probe_dt_alias() but not properly
> solved.
>
> Also keep status of free ids/minor numbers in bitmap to know what's the
> next unallocated number.
>
> The same solution can be used in pl011, uart16550 and uartlite to
> really
> get unified kernel.
>
> Tested on these use cases:
> Notes:
> ff000000 is the first PS uart listed in dtsi
> ff010000 is the second PS uart listed in dtsi
>
> Standard zcu102 setting
> serial0 = &uart0;
> serial1 = &uart1;
> [ 0.196628] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.642542] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> Swapped zcu102 setting
> serial0 = &uart1;
> serial1 = &uart0;
> [ 0.196472] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.196824] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> uart0 on alias higher then MAX uart ports
> serial0 = &uart1;
> serial200 = &uart0;
> [ 0.176793] ff000000.serial: ttyPS200 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.177288] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> Both uarts on higher aliases
> serial1 = &uart1;
> serial2 = &uart0;
> [ 0.196470] ff000000.serial: ttyPS2 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.196823] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> uart0 not listed but it is probed first that's why should be ttyPS0 but
> there is uart1 via alias
> serial0 = &uart1;
> [ 0.176656] xuartps ff000000.serial: No serial alias passed. Using
> the first free id
> [ 0.176671] xuartps ff000000.serial: Validate id 0
> [ 0.176680] xuartps ff000000.serial: The empty id is 0
> [ 0.176692] xuartps ff000000.serial: ID 0 already taken - skipped
> [ 0.176701] xuartps ff000000.serial: Validate id 1
> [ 0.176710] xuartps ff000000.serial: The empty id is 1
> [ 0.176719] xuartps ff000000.serial: Selected ID 1 allocation passed
> [ 0.176760] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.177104] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> uart0 not listed but it is probed first that's why should be ttyPS0
> serial1 = &uart1;
> [ 0.176661] xuartps ff000000.serial: No serial alias passed. Using
> the first free id
> [ 0.176676] xuartps ff000000.serial: Validate id 0
> [ 0.176686] xuartps ff000000.serial: The empty id is 0
> [ 0.176696] xuartps ff000000.serial: Selected ID 0 allocation passed
> [ 0.176737] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.177069] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> uarts not listed in aliases list
> [ 0.176673] xuartps ff000000.serial: No serial alias passed. Using
> the first free id
> [ 0.176687] xuartps ff000000.serial: Validate id 0
> [ 0.176697] xuartps ff000000.serial: The empty id is 0
> [ 0.176707] xuartps ff000000.serial: Selected ID 0 allocation passed
> [ 0.176746] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
> base_baud = 6250000) is a xuartps
> [ 0.177057] xuartps ff010000.serial: No serial alias passed. Using
> the first free id
> [ 0.177070] xuartps ff010000.serial: Validate id 0
> [ 0.177079] xuartps ff010000.serial: The empty id is 0
> [ 0.177089] xuartps ff010000.serial: Selected ID 0 allocation failed
> [ 0.177098] xuartps ff010000.serial: Validate id 1
> [ 0.177107] xuartps ff010000.serial: The empty id is 1
> [ 0.177116] xuartps ff010000.serial: Selected ID 1 allocation passed
> [ 0.177149] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
> base_baud = 6250000) is a xuartps
>
> Thanks,
> Michal
Hello Michal,
How will this interact with ns16550 based UARTs?
Can we have both /dev/ttyS0 and /dev/ttyPS0?
Currently we can't unless we create *no* serialN alias for the ns16550.
And there is no other means to lock the /dev/ttySx name to a device
either.
Or will the xuartps driver eventually use /dev/ttySx as well?
Maarten
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 0/3] serial: uartps: Add run time support for more IPs than hardcoded 2
2018-05-05 13:10 ` Maarten Brock
(?)
@ 2018-05-09 12:09 ` Michal Simek
-1 siblings, 0 replies; 22+ messages in thread
From: Michal Simek @ 2018-05-09 12:09 UTC (permalink / raw)
To: Maarten Brock, Michal Simek
Cc: linux-kernel, monstr, gnomes, Alexander Graf, devicetree,
Jiri Slaby, Greg Kroah-Hartman, Rob Herring, linux-serial,
Frank Rowand, linux-arm-kernel, linux-serial-owner
Hi,
On 5.5.2018 15:10, Maarten Brock wrote:
> On 2018-04-26 16:08, Michal Simek wrote:
>> Hi,
>>
>> this series is trying to address discussion I had with Alan in past
>> https://patchwork.kernel.org/patch/9738445/.
>>
>> It is moving uart_register_driver() to probe function like it is done in
>> pl011 driver.
>> And also introducing new function for alias compatibility checking to
>> resolve cases where some IPs have alias and some of them not.
>> This case is detected in pl011_probe_dt_alias() but not properly solved.
>>
>> Also keep status of free ids/minor numbers in bitmap to know what's the
>> next unallocated number.
>>
>> The same solution can be used in pl011, uart16550 and uartlite to really
>> get unified kernel.
>>
>> Tested on these use cases:
>> Notes:
>> ff000000 is the first PS uart listed in dtsi
>> ff010000 is the second PS uart listed in dtsi
>>
>> Standard zcu102 setting
>> serial0 = &uart0;
>> serial1 = &uart1;
>> [ 0.196628] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.642542] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> Swapped zcu102 setting
>> serial0 = &uart1;
>> serial1 = &uart0;
>> [ 0.196472] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.196824] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> uart0 on alias higher then MAX uart ports
>> serial0 = &uart1;
>> serial200 = &uart0;
>> [ 0.176793] ff000000.serial: ttyPS200 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.177288] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> Both uarts on higher aliases
>> serial1 = &uart1;
>> serial2 = &uart0;
>> [ 0.196470] ff000000.serial: ttyPS2 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.196823] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> uart0 not listed but it is probed first that's why should be ttyPS0 but
>> there is uart1 via alias
>> serial0 = &uart1;
>> [ 0.176656] xuartps ff000000.serial: No serial alias passed. Using
>> the first free id
>> [ 0.176671] xuartps ff000000.serial: Validate id 0
>> [ 0.176680] xuartps ff000000.serial: The empty id is 0
>> [ 0.176692] xuartps ff000000.serial: ID 0 already taken - skipped
>> [ 0.176701] xuartps ff000000.serial: Validate id 1
>> [ 0.176710] xuartps ff000000.serial: The empty id is 1
>> [ 0.176719] xuartps ff000000.serial: Selected ID 1 allocation passed
>> [ 0.176760] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.177104] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> uart0 not listed but it is probed first that's why should be ttyPS0
>> serial1 = &uart1;
>> [ 0.176661] xuartps ff000000.serial: No serial alias passed. Using
>> the first free id
>> [ 0.176676] xuartps ff000000.serial: Validate id 0
>> [ 0.176686] xuartps ff000000.serial: The empty id is 0
>> [ 0.176696] xuartps ff000000.serial: Selected ID 0 allocation passed
>> [ 0.176737] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.177069] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> uarts not listed in aliases list
>> [ 0.176673] xuartps ff000000.serial: No serial alias passed. Using
>> the first free id
>> [ 0.176687] xuartps ff000000.serial: Validate id 0
>> [ 0.176697] xuartps ff000000.serial: The empty id is 0
>> [ 0.176707] xuartps ff000000.serial: Selected ID 0 allocation passed
>> [ 0.176746] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.177057] xuartps ff010000.serial: No serial alias passed. Using
>> the first free id
>> [ 0.177070] xuartps ff010000.serial: Validate id 0
>> [ 0.177079] xuartps ff010000.serial: The empty id is 0
>> [ 0.177089] xuartps ff010000.serial: Selected ID 0 allocation failed
>> [ 0.177098] xuartps ff010000.serial: Validate id 1
>> [ 0.177107] xuartps ff010000.serial: The empty id is 1
>> [ 0.177116] xuartps ff010000.serial: Selected ID 1 allocation passed
>> [ 0.177149] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> Thanks,
>> Michal
>
> Hello Michal,
>
> How will this interact with ns16550 based UARTs?
>
> Can we have both /dev/ttyS0 and /dev/ttyPS0?
> Currently we can't unless we create *no* serialN alias for the ns16550.
> And there is no other means to lock the /dev/ttySx name to a device either.
>
> Or will the xuartps driver eventually use /dev/ttySx as well?
I am not changing any current behavior in this case. uartps is using
ttyPS. ns16550 is using ttyS and on xilinx devices we are using also
ttyUL for uartlite. Other drivers are using different names.
Thanks,
Michal
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH 0/3] serial: uartps: Add run time support for more IPs than hardcoded 2
@ 2018-05-09 12:09 ` Michal Simek
0 siblings, 0 replies; 22+ messages in thread
From: Michal Simek @ 2018-05-09 12:09 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 5.5.2018 15:10, Maarten Brock wrote:
> On 2018-04-26 16:08, Michal Simek wrote:
>> Hi,
>>
>> this series is trying to address discussion I had with Alan in past
>> https://patchwork.kernel.org/patch/9738445/.
>>
>> It is moving uart_register_driver() to probe function like it is done in
>> pl011 driver.
>> And also introducing new function for alias compatibility checking to
>> resolve cases where some IPs have alias and some of them not.
>> This case is detected in pl011_probe_dt_alias() but not properly solved.
>>
>> Also keep status of free ids/minor numbers in bitmap to know what's the
>> next unallocated number.
>>
>> The same solution can be used in pl011, uart16550 and uartlite to really
>> get unified kernel.
>>
>> Tested on these use cases:
>> Notes:
>> ff000000 is the first PS uart listed in dtsi
>> ff010000 is the second PS uart listed in dtsi
>>
>> Standard zcu102 setting
>> ??????? serial0 = &uart0;
>> ??????? serial1 = &uart1;
>> [??? 0.196628] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [??? 0.642542] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> Swapped zcu102 setting
>> ??????? serial0 = &uart1;
>> ??????? serial1 = &uart0;
>> [??? 0.196472] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [??? 0.196824] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> uart0 on alias higher then MAX uart ports
>> ??????? serial0 = &uart1;
>> ??????? serial200 = &uart0;
>> [??? 0.176793] ff000000.serial: ttyPS200 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [??? 0.177288] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> Both uarts on higher aliases
>> ??????? serial1 = &uart1;
>> ??????? serial2 = &uart0;
>> [??? 0.196470] ff000000.serial: ttyPS2 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [??? 0.196823] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> uart0 not listed but it is probed first that's why should be ttyPS0 but
>> there is uart1 via alias
>> ??????? serial0 = &uart1;
>> [??? 0.176656] xuartps ff000000.serial: No serial alias passed. Using
>> the first free id
>> [??? 0.176671] xuartps ff000000.serial: Validate id 0
>> [??? 0.176680] xuartps ff000000.serial: The empty id is 0
>> [??? 0.176692] xuartps ff000000.serial: ID 0 already taken - skipped
>> [??? 0.176701] xuartps ff000000.serial: Validate id 1
>> [??? 0.176710] xuartps ff000000.serial: The empty id is 1
>> [??? 0.176719] xuartps ff000000.serial: Selected ID 1 allocation passed
>> [??? 0.176760] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [??? 0.177104] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> uart0 not listed but it is probed first that's why should be ttyPS0
>> ??????? serial1 = &uart1;
>> [??? 0.176661] xuartps ff000000.serial: No serial alias passed. Using
>> the first free id
>> [??? 0.176676] xuartps ff000000.serial: Validate id 0
>> [??? 0.176686] xuartps ff000000.serial: The empty id is 0
>> [??? 0.176696] xuartps ff000000.serial: Selected ID 0 allocation passed
>> [??? 0.176737] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [??? 0.177069] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> uarts not listed in aliases list
>> [??? 0.176673] xuartps ff000000.serial: No serial alias passed. Using
>> the first free id
>> [??? 0.176687] xuartps ff000000.serial: Validate id 0
>> [??? 0.176697] xuartps ff000000.serial: The empty id is 0
>> [??? 0.176707] xuartps ff000000.serial: Selected ID 0 allocation passed
>> [??? 0.176746] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [??? 0.177057] xuartps ff010000.serial: No serial alias passed. Using
>> the first free id
>> [??? 0.177070] xuartps ff010000.serial: Validate id 0
>> [??? 0.177079] xuartps ff010000.serial: The empty id is 0
>> [??? 0.177089] xuartps ff010000.serial: Selected ID 0 allocation failed
>> [??? 0.177098] xuartps ff010000.serial: Validate id 1
>> [??? 0.177107] xuartps ff010000.serial: The empty id is 1
>> [??? 0.177116] xuartps ff010000.serial: Selected ID 1 allocation passed
>> [??? 0.177149] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> Thanks,
>> Michal
>
> Hello Michal,
>
> How will this interact with ns16550 based UARTs?
>
> Can we have both /dev/ttyS0 and /dev/ttyPS0?
> Currently we can't unless we create *no* serialN alias for the ns16550.
> And there is no other means to lock the /dev/ttySx name to a device either.
>
> Or will the xuartps driver eventually use /dev/ttySx as well?
I am not changing any current behavior in this case. uartps is using
ttyPS. ns16550 is using ttyS and on xilinx devices we are using also
ttyUL for uartlite. Other drivers are using different names.
Thanks,
Michal
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 0/3] serial: uartps: Add run time support for more IPs than hardcoded 2
@ 2018-05-09 12:09 ` Michal Simek
0 siblings, 0 replies; 22+ messages in thread
From: Michal Simek @ 2018-05-09 12:09 UTC (permalink / raw)
To: Maarten Brock, Michal Simek
Cc: linux-kernel, monstr, gnomes, Alexander Graf, devicetree,
Jiri Slaby, Greg Kroah-Hartman, Rob Herring, linux-serial,
Frank Rowand, linux-arm-kernel, linux-serial-owner
Hi,
On 5.5.2018 15:10, Maarten Brock wrote:
> On 2018-04-26 16:08, Michal Simek wrote:
>> Hi,
>>
>> this series is trying to address discussion I had with Alan in past
>> https://patchwork.kernel.org/patch/9738445/.
>>
>> It is moving uart_register_driver() to probe function like it is done in
>> pl011 driver.
>> And also introducing new function for alias compatibility checking to
>> resolve cases where some IPs have alias and some of them not.
>> This case is detected in pl011_probe_dt_alias() but not properly solved.
>>
>> Also keep status of free ids/minor numbers in bitmap to know what's the
>> next unallocated number.
>>
>> The same solution can be used in pl011, uart16550 and uartlite to really
>> get unified kernel.
>>
>> Tested on these use cases:
>> Notes:
>> ff000000 is the first PS uart listed in dtsi
>> ff010000 is the second PS uart listed in dtsi
>>
>> Standard zcu102 setting
>> serial0 = &uart0;
>> serial1 = &uart1;
>> [ 0.196628] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.642542] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> Swapped zcu102 setting
>> serial0 = &uart1;
>> serial1 = &uart0;
>> [ 0.196472] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.196824] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> uart0 on alias higher then MAX uart ports
>> serial0 = &uart1;
>> serial200 = &uart0;
>> [ 0.176793] ff000000.serial: ttyPS200 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.177288] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> Both uarts on higher aliases
>> serial1 = &uart1;
>> serial2 = &uart0;
>> [ 0.196470] ff000000.serial: ttyPS2 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.196823] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> uart0 not listed but it is probed first that's why should be ttyPS0 but
>> there is uart1 via alias
>> serial0 = &uart1;
>> [ 0.176656] xuartps ff000000.serial: No serial alias passed. Using
>> the first free id
>> [ 0.176671] xuartps ff000000.serial: Validate id 0
>> [ 0.176680] xuartps ff000000.serial: The empty id is 0
>> [ 0.176692] xuartps ff000000.serial: ID 0 already taken - skipped
>> [ 0.176701] xuartps ff000000.serial: Validate id 1
>> [ 0.176710] xuartps ff000000.serial: The empty id is 1
>> [ 0.176719] xuartps ff000000.serial: Selected ID 1 allocation passed
>> [ 0.176760] ff000000.serial: ttyPS1 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.177104] ff010000.serial: ttyPS0 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> uart0 not listed but it is probed first that's why should be ttyPS0
>> serial1 = &uart1;
>> [ 0.176661] xuartps ff000000.serial: No serial alias passed. Using
>> the first free id
>> [ 0.176676] xuartps ff000000.serial: Validate id 0
>> [ 0.176686] xuartps ff000000.serial: The empty id is 0
>> [ 0.176696] xuartps ff000000.serial: Selected ID 0 allocation passed
>> [ 0.176737] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.177069] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> uarts not listed in aliases list
>> [ 0.176673] xuartps ff000000.serial: No serial alias passed. Using
>> the first free id
>> [ 0.176687] xuartps ff000000.serial: Validate id 0
>> [ 0.176697] xuartps ff000000.serial: The empty id is 0
>> [ 0.176707] xuartps ff000000.serial: Selected ID 0 allocation passed
>> [ 0.176746] ff000000.serial: ttyPS0 at MMIO 0xff000000 (irq = 33,
>> base_baud = 6250000) is a xuartps
>> [ 0.177057] xuartps ff010000.serial: No serial alias passed. Using
>> the first free id
>> [ 0.177070] xuartps ff010000.serial: Validate id 0
>> [ 0.177079] xuartps ff010000.serial: The empty id is 0
>> [ 0.177089] xuartps ff010000.serial: Selected ID 0 allocation failed
>> [ 0.177098] xuartps ff010000.serial: Validate id 1
>> [ 0.177107] xuartps ff010000.serial: The empty id is 1
>> [ 0.177116] xuartps ff010000.serial: Selected ID 1 allocation passed
>> [ 0.177149] ff010000.serial: ttyPS1 at MMIO 0xff010000 (irq = 34,
>> base_baud = 6250000) is a xuartps
>>
>> Thanks,
>> Michal
>
> Hello Michal,
>
> How will this interact with ns16550 based UARTs?
>
> Can we have both /dev/ttyS0 and /dev/ttyPS0?
> Currently we can't unless we create *no* serialN alias for the ns16550.
> And there is no other means to lock the /dev/ttySx name to a device either.
>
> Or will the xuartps driver eventually use /dev/ttySx as well?
I am not changing any current behavior in this case. uartps is using
ttyPS. ns16550 is using ttyS and on xilinx devices we are using also
ttyUL for uartlite. Other drivers are using different names.
Thanks,
Michal
^ permalink raw reply [flat|nested] 22+ messages in thread