Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
Hi Andrew,
> Am 16.11.2017 um 19:56 schrieb H. Nikolaus Schaller :
>
> Hi Andrew,
>
>> Am 16.11.2017 um 19:32 schrieb Andrew F. Davis :
>>
>> On 11/16/2017 12:18 PM, H. Nikolaus Schaller wrote:
>>>
Am 16.11.2017 um 18:08 schrieb Andrew F. Davis :
On 11/16/2017 10:10 AM, H. Nikolaus Schaller wrote:
> Hi Andrew,
>
>> Am 16.11.2017 um 16:53 schrieb Andrew F. Davis :
>>
>> On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
>>>
Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
On 16/11/17 10:50, H. Nikolaus Schaller wrote:
> The vendor name was "toppoly" but other panels and the vendor list
> have defined it as "tpo". So let's fix it in driver and bindings.
>
> Signed-off-by: H. Nikolaus Schaller
> ---
> -MODULE_ALIAS("spi:toppoly,td028ttec1");
> +MODULE_ALIAS("spi:tpo,td028ttec1");
Doesn't this mean that the module won't load if you have old bindings?
>>>
>>> Hm.
>>>
>>> Well, I think it can load but doesn't automatically from DT strings
>>> which might
>>> be unexpected.
>>>
Can't we have two module aliases?
>>>
>>> I think we can. Just a random example:
>>> https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
>>>
>>> So we should keep both.
>>
>> Even better would be to drop both MODULE_ALIAS and let the
>> MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
>
> Why would that be better?
>
MODULE_ALIAS is ugly, you already have a table (usually) of device names
that are supported by the driver, the module aliases should be generated
from that table. This also keeps supported device list in one place.
> As far as I see it will need more code and changes than adding one line of
> MODULE_ALIAS.
>
>> Although it doesn't look like this driver has an SPI id table, you
>> should probably add one, I be interested to see if this module is always
>> being matched through the "spi" or the "of" alias..
>
> Could you please propose how that code should look like, so that I can
> test?
>
Sure,
start with
$ udevadm monitor
and see what string the kernel is looking for when trying to find a
module for this device.
>>>
>>> Well, the module is loaded automatically from DT at boot time well before
>>> I can start udevadm. So that is the most tricky part to setup the system
>>> to suppress this...
>>>
If it is only ever looking for "of:toppoly,td028ttec1", then you can
drop the MODULE_ALIAS and be done as it was never getting used anyway.
>>>
>>> Since it is an SPI client, I am sure it looks for "spi:something.
>>>
What I expect though is "spi:toppoly,td028ttec1", in which case you
should add
static const struct spi_device_id td028ttec1_ids[] = {
{ "toppoly,td028ttec1", 0 },
{ "tpo,td028ttec1", 0},
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(spi, td028ttec1_ids);
>>>
>>> We already have a static const struct of_device_id td028ttec1_of_match[]
>>> table with the same information.
>>>
>>> So we still have two places to keep in sync.
>>>
>>> Or can we remove the td028ttec1_of_match[]? AFAIK not.
>>>
link to it in the td028ttec1_spi_driver struct:
.id_table = td028ttec1_ids,
Then test again to see that the module still loads with the new and old
DT string.
>>>
>>> In total I am not really convinced that adding 7 lines of code is better
>>> than one (the "tpo," alias) that is tested and works...
>>>
>>> And it looks like a lot of unplanned code testing for me which takes more
>>> than 5 minutes :)
>>>
>>> So I'd prefer to leave that exercise of fixing the MODULE_ALIAS/DEVICE_TABLE
>>> to someone else...
>>>
>>
>> That's fine, someday I'll probably get some script to do this for all
>> the drivers that still have MODULE_ALIAS and an existing table.
>
> That would be cool!
>
> On a second thought, I think there is a quick experiment for this driver
> not needing to monitor events.
>
> 1st attempt: remove ALIASES => if it still loads it would be fine
> 2nd attempt: add your id table => if it loads again, it is fine
> if not, let's keep ALIASES.
>
> Maybe I can try tomorrow.
I found time to give it a try and indeed:
1. with module_aliases
root@letux:~# lsmod | fgrep td028
panel_tpo_td028ttec116384 1
omapdss_base 16384 5
connector_analog_tv,encoder_opa362,panel_tpo_td028ttec1,omapdrm,omapdss
root@letux:~# modprobe -c | fgrep td028
alias of:N*T*Comapdss,toppoly,td028ttec1 panel_tpo_td028ttec1
alias of:N*T*Comapdss,toppoly,td028ttec1C* panel_tpo_td028ttec1
alias of:N*T*Comapdss,tpo,td028ttec1 panel_tpo_td028ttec1
alias of:N*T*Comapdss,tpo,td028ttec1C* panel_tpo_td028ttec1
alias spi:
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
Hi Andrew,
> Am 16.11.2017 um 19:32 schrieb Andrew F. Davis :
>
> On 11/16/2017 12:18 PM, H. Nikolaus Schaller wrote:
>>
>>> Am 16.11.2017 um 18:08 schrieb Andrew F. Davis :
>>>
>>> On 11/16/2017 10:10 AM, H. Nikolaus Schaller wrote:
Hi Andrew,
> Am 16.11.2017 um 16:53 schrieb Andrew F. Davis :
>
> On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
>>
>>> Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
>>>
>>> On 16/11/17 10:50, H. Nikolaus Schaller wrote:
The vendor name was "toppoly" but other panels and the vendor list
have defined it as "tpo". So let's fix it in driver and bindings.
Signed-off-by: H. Nikolaus Schaller
---
>>>
>>>
-MODULE_ALIAS("spi:toppoly,td028ttec1");
+MODULE_ALIAS("spi:tpo,td028ttec1");
>>>
>>> Doesn't this mean that the module won't load if you have old bindings?
>>
>> Hm.
>>
>> Well, I think it can load but doesn't automatically from DT strings
>> which might
>> be unexpected.
>>
>>> Can't we have two module aliases?
>>
>> I think we can. Just a random example:
>> https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
>>
>> So we should keep both.
>
> Even better would be to drop both MODULE_ALIAS and let the
> MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
Why would that be better?
>>>
>>> MODULE_ALIAS is ugly, you already have a table (usually) of device names
>>> that are supported by the driver, the module aliases should be generated
>>> from that table. This also keeps supported device list in one place.
>>>
As far as I see it will need more code and changes than adding one line of
MODULE_ALIAS.
> Although it doesn't look like this driver has an SPI id table, you
> should probably add one, I be interested to see if this module is always
> being matched through the "spi" or the "of" alias..
Could you please propose how that code should look like, so that I can
test?
>>>
>>> Sure,
>>>
>>> start with
>>> $ udevadm monitor
>>> and see what string the kernel is looking for when trying to find a
>>> module for this device.
>>
>> Well, the module is loaded automatically from DT at boot time well before
>> I can start udevadm. So that is the most tricky part to setup the system
>> to suppress this...
>>
>>>
>>> If it is only ever looking for "of:toppoly,td028ttec1", then you can
>>> drop the MODULE_ALIAS and be done as it was never getting used anyway.
>>
>> Since it is an SPI client, I am sure it looks for "spi:something.
>>
>>>
>>> What I expect though is "spi:toppoly,td028ttec1", in which case you
>>> should add
>>>
>>> static const struct spi_device_id td028ttec1_ids[] = {
>>> { "toppoly,td028ttec1", 0 },
>>> { "tpo,td028ttec1", 0},
>>> { /* sentinel */ }
>>> };
>>> MODULE_DEVICE_TABLE(spi, td028ttec1_ids);
>>
>> We already have a static const struct of_device_id td028ttec1_of_match[]
>> table with the same information.
>>
>> So we still have two places to keep in sync.
>>
>> Or can we remove the td028ttec1_of_match[]? AFAIK not.
>>
>>>
>>> link to it in the td028ttec1_spi_driver struct:
>>> .id_table = td028ttec1_ids,
>>>
>>> Then test again to see that the module still loads with the new and old
>>> DT string.
>>
>> In total I am not really convinced that adding 7 lines of code is better
>> than one (the "tpo," alias) that is tested and works...
>>
>> And it looks like a lot of unplanned code testing for me which takes more
>> than 5 minutes :)
>>
>> So I'd prefer to leave that exercise of fixing the MODULE_ALIAS/DEVICE_TABLE
>> to someone else...
>>
>
> That's fine, someday I'll probably get some script to do this for all
> the drivers that still have MODULE_ALIAS and an existing table.
That would be cool!
On a second thought, I think there is a quick experiment for this driver
not needing to monitor events.
1st attempt: remove ALIASES => if it still loads it would be fine
2nd attempt: add your id table => if it loads again, it is fine
if not, let's keep ALIASES.
Maybe I can try tomorrow.
BR and thanks,
Nikolaus
>
>> BR and thanks,
>> Nikolaus
>>
>>>
>>> Andrew
>>>
BR and thanks,
Nikolaus Schaller
>
>>
>> Should I submit a new version?
>>
>> BR,
>> Nikolaus
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> --
>> To unsubscribe from this list: send the line "u
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
On 11/16/2017 12:18 PM, H. Nikolaus Schaller wrote:
>
>> Am 16.11.2017 um 18:08 schrieb Andrew F. Davis :
>>
>> On 11/16/2017 10:10 AM, H. Nikolaus Schaller wrote:
>>> Hi Andrew,
>>>
Am 16.11.2017 um 16:53 schrieb Andrew F. Davis :
On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
>
>> Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
>>
>> On 16/11/17 10:50, H. Nikolaus Schaller wrote:
>>> The vendor name was "toppoly" but other panels and the vendor list
>>> have defined it as "tpo". So let's fix it in driver and bindings.
>>>
>>> Signed-off-by: H. Nikolaus Schaller
>>> ---
>>
>>
>>> -MODULE_ALIAS("spi:toppoly,td028ttec1");
>>> +MODULE_ALIAS("spi:tpo,td028ttec1");
>>
>> Doesn't this mean that the module won't load if you have old bindings?
>
> Hm.
>
> Well, I think it can load but doesn't automatically from DT strings which
> might
> be unexpected.
>
>> Can't we have two module aliases?
>
> I think we can. Just a random example:
> https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
>
> So we should keep both.
Even better would be to drop both MODULE_ALIAS and let the
MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
>>>
>>> Why would that be better?
>>>
>>
>> MODULE_ALIAS is ugly, you already have a table (usually) of device names
>> that are supported by the driver, the module aliases should be generated
>> from that table. This also keeps supported device list in one place.
>>
>>> As far as I see it will need more code and changes than adding one line of
>>> MODULE_ALIAS.
>>>
Although it doesn't look like this driver has an SPI id table, you
should probably add one, I be interested to see if this module is always
being matched through the "spi" or the "of" alias..
>>>
>>> Could you please propose how that code should look like, so that I can test?
>>>
>>
>> Sure,
>>
>> start with
>> $ udevadm monitor
>> and see what string the kernel is looking for when trying to find a
>> module for this device.
>
> Well, the module is loaded automatically from DT at boot time well before
> I can start udevadm. So that is the most tricky part to setup the system
> to suppress this...
>
>>
>> If it is only ever looking for "of:toppoly,td028ttec1", then you can
>> drop the MODULE_ALIAS and be done as it was never getting used anyway.
>
> Since it is an SPI client, I am sure it looks for "spi:something.
>
>>
>> What I expect though is "spi:toppoly,td028ttec1", in which case you
>> should add
>>
>> static const struct spi_device_id td028ttec1_ids[] = {
>> { "toppoly,td028ttec1", 0 },
>> { "tpo,td028ttec1", 0},
>> { /* sentinel */ }
>> };
>> MODULE_DEVICE_TABLE(spi, td028ttec1_ids);
>
> We already have a static const struct of_device_id td028ttec1_of_match[]
> table with the same information.
>
> So we still have two places to keep in sync.
>
> Or can we remove the td028ttec1_of_match[]? AFAIK not.
>
>>
>> link to it in the td028ttec1_spi_driver struct:
>> .id_table = td028ttec1_ids,
>>
>> Then test again to see that the module still loads with the new and old
>> DT string.
>
> In total I am not really convinced that adding 7 lines of code is better
> than one (the "tpo," alias) that is tested and works...
>
> And it looks like a lot of unplanned code testing for me which takes more
> than 5 minutes :)
>
> So I'd prefer to leave that exercise of fixing the MODULE_ALIAS/DEVICE_TABLE
> to someone else...
>
That's fine, someday I'll probably get some script to do this for all
the drivers that still have MODULE_ALIAS and an existing table.
> BR and thanks,
> Nikolaus
>
>>
>> Andrew
>>
>>> BR and thanks,
>>> Nikolaus Schaller
>>>
>
> Should I submit a new version?
>
> BR,
> Nikolaus
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
> Am 16.11.2017 um 18:08 schrieb Andrew F. Davis :
>
> On 11/16/2017 10:10 AM, H. Nikolaus Schaller wrote:
>> Hi Andrew,
>>
>>> Am 16.11.2017 um 16:53 schrieb Andrew F. Davis :
>>>
>>> On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
> Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
>
> On 16/11/17 10:50, H. Nikolaus Schaller wrote:
>> The vendor name was "toppoly" but other panels and the vendor list
>> have defined it as "tpo". So let's fix it in driver and bindings.
>>
>> Signed-off-by: H. Nikolaus Schaller
>> ---
>
>
>> -MODULE_ALIAS("spi:toppoly,td028ttec1");
>> +MODULE_ALIAS("spi:tpo,td028ttec1");
>
> Doesn't this mean that the module won't load if you have old bindings?
Hm.
Well, I think it can load but doesn't automatically from DT strings which
might
be unexpected.
> Can't we have two module aliases?
I think we can. Just a random example:
https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
So we should keep both.
>>>
>>> Even better would be to drop both MODULE_ALIAS and let the
>>> MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
>>
>> Why would that be better?
>>
>
> MODULE_ALIAS is ugly, you already have a table (usually) of device names
> that are supported by the driver, the module aliases should be generated
> from that table. This also keeps supported device list in one place.
>
>> As far as I see it will need more code and changes than adding one line of
>> MODULE_ALIAS.
>>
>>> Although it doesn't look like this driver has an SPI id table, you
>>> should probably add one, I be interested to see if this module is always
>>> being matched through the "spi" or the "of" alias..
>>
>> Could you please propose how that code should look like, so that I can test?
>>
>
> Sure,
>
> start with
> $ udevadm monitor
> and see what string the kernel is looking for when trying to find a
> module for this device.
Well, the module is loaded automatically from DT at boot time well before
I can start udevadm. So that is the most tricky part to setup the system
to suppress this...
>
> If it is only ever looking for "of:toppoly,td028ttec1", then you can
> drop the MODULE_ALIAS and be done as it was never getting used anyway.
Since it is an SPI client, I am sure it looks for "spi:something.
>
> What I expect though is "spi:toppoly,td028ttec1", in which case you
> should add
>
> static const struct spi_device_id td028ttec1_ids[] = {
> { "toppoly,td028ttec1", 0 },
> { "tpo,td028ttec1", 0},
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(spi, td028ttec1_ids);
We already have a static const struct of_device_id td028ttec1_of_match[]
table with the same information.
So we still have two places to keep in sync.
Or can we remove the td028ttec1_of_match[]? AFAIK not.
>
> link to it in the td028ttec1_spi_driver struct:
> .id_table = td028ttec1_ids,
>
> Then test again to see that the module still loads with the new and old
> DT string.
In total I am not really convinced that adding 7 lines of code is better
than one (the "tpo," alias) that is tested and works...
And it looks like a lot of unplanned code testing for me which takes more
than 5 minutes :)
So I'd prefer to leave that exercise of fixing the MODULE_ALIAS/DEVICE_TABLE
to someone else...
BR and thanks,
Nikolaus
>
> Andrew
>
>> BR and thanks,
>> Nikolaus Schaller
>>
>>>
Should I submit a new version?
BR,
Nikolaus
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
On 11/16/2017 10:10 AM, H. Nikolaus Schaller wrote:
> Hi Andrew,
>
>> Am 16.11.2017 um 16:53 schrieb Andrew F. Davis :
>>
>> On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
>>>
Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
On 16/11/17 10:50, H. Nikolaus Schaller wrote:
> The vendor name was "toppoly" but other panels and the vendor list
> have defined it as "tpo". So let's fix it in driver and bindings.
>
> Signed-off-by: H. Nikolaus Schaller
> ---
> -MODULE_ALIAS("spi:toppoly,td028ttec1");
> +MODULE_ALIAS("spi:tpo,td028ttec1");
Doesn't this mean that the module won't load if you have old bindings?
>>>
>>> Hm.
>>>
>>> Well, I think it can load but doesn't automatically from DT strings which
>>> might
>>> be unexpected.
>>>
Can't we have two module aliases?
>>>
>>> I think we can. Just a random example:
>>> https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
>>>
>>> So we should keep both.
>>
>> Even better would be to drop both MODULE_ALIAS and let the
>> MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
>
> Why would that be better?
>
MODULE_ALIAS is ugly, you already have a table (usually) of device names
that are supported by the driver, the module aliases should be generated
from that table. This also keeps supported device list in one place.
> As far as I see it will need more code and changes than adding one line of
> MODULE_ALIAS.
>
>> Although it doesn't look like this driver has an SPI id table, you
>> should probably add one, I be interested to see if this module is always
>> being matched through the "spi" or the "of" alias..
>
> Could you please propose how that code should look like, so that I can test?
>
Sure,
start with
$ udevadm monitor
and see what string the kernel is looking for when trying to find a
module for this device.
If it is only ever looking for "of:toppoly,td028ttec1", then you can
drop the MODULE_ALIAS and be done as it was never getting used anyway.
What I expect though is "spi:toppoly,td028ttec1", in which case you
should add
static const struct spi_device_id td028ttec1_ids[] = {
{ "toppoly,td028ttec1", 0 },
{ "tpo,td028ttec1", 0},
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(spi, td028ttec1_ids);
link to it in the td028ttec1_spi_driver struct:
.id_table = td028ttec1_ids,
Then test again to see that the module still loads with the new and old
DT string.
Andrew
> BR and thanks,
> Nikolaus Schaller
>
>>
>>>
>>> Should I submit a new version?
>>>
>>> BR,
>>> Nikolaus
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
Hi Andrew,
> Am 16.11.2017 um 16:53 schrieb Andrew F. Davis :
>
> On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
>>
>>> Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
>>>
>>> On 16/11/17 10:50, H. Nikolaus Schaller wrote:
The vendor name was "toppoly" but other panels and the vendor list
have defined it as "tpo". So let's fix it in driver and bindings.
Signed-off-by: H. Nikolaus Schaller
---
>>>
>>>
-MODULE_ALIAS("spi:toppoly,td028ttec1");
+MODULE_ALIAS("spi:tpo,td028ttec1");
>>>
>>> Doesn't this mean that the module won't load if you have old bindings?
>>
>> Hm.
>>
>> Well, I think it can load but doesn't automatically from DT strings which
>> might
>> be unexpected.
>>
>>> Can't we have two module aliases?
>>
>> I think we can. Just a random example:
>> https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
>>
>> So we should keep both.
>
> Even better would be to drop both MODULE_ALIAS and let the
> MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
Why would that be better?
As far as I see it will need more code and changes than adding one line of
MODULE_ALIAS.
> Although it doesn't look like this driver has an SPI id table, you
> should probably add one, I be interested to see if this module is always
> being matched through the "spi" or the "of" alias..
Could you please propose how that code should look like, so that I can test?
BR and thanks,
Nikolaus Schaller
>
>>
>> Should I submit a new version?
>>
>> BR,
>> Nikolaus
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
>
>> Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
>>
>> On 16/11/17 10:50, H. Nikolaus Schaller wrote:
>>> The vendor name was "toppoly" but other panels and the vendor list
>>> have defined it as "tpo". So let's fix it in driver and bindings.
>>>
>>> Signed-off-by: H. Nikolaus Schaller
>>> ---
>>
>>
>>> -MODULE_ALIAS("spi:toppoly,td028ttec1");
>>> +MODULE_ALIAS("spi:tpo,td028ttec1");
>>
>> Doesn't this mean that the module won't load if you have old bindings?
>
> Hm.
>
> Well, I think it can load but doesn't automatically from DT strings which
> might
> be unexpected.
>
>> Can't we have two module aliases?
>
> I think we can. Just a random example:
> https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
>
> So we should keep both.
Even better would be to drop both MODULE_ALIAS and let the
MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
Although it doesn't look like this driver has an SPI id table, you
should probably add one, I be interested to see if this module is always
being matched through the "spi" or the "of" alias..
>
> Should I submit a new version?
>
> BR,
> Nikolaus
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
> Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
>
> On 16/11/17 10:50, H. Nikolaus Schaller wrote:
>> The vendor name was "toppoly" but other panels and the vendor list
>> have defined it as "tpo". So let's fix it in driver and bindings.
>>
>> Signed-off-by: H. Nikolaus Schaller
>> ---
>
>
>> -MODULE_ALIAS("spi:toppoly,td028ttec1");
>> +MODULE_ALIAS("spi:tpo,td028ttec1");
>
> Doesn't this mean that the module won't load if you have old bindings?
Hm.
Well, I think it can load but doesn't automatically from DT strings which might
be unexpected.
> Can't we have two module aliases?
I think we can. Just a random example:
https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
So we should keep both.
Should I submit a new version?
BR,
Nikolaus
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
On 16/11/17 10:50, H. Nikolaus Schaller wrote:
> The vendor name was "toppoly" but other panels and the vendor list
> have defined it as "tpo". So let's fix it in driver and bindings.
>
> Signed-off-by: H. Nikolaus Schaller
> ---
> -MODULE_ALIAS("spi:toppoly,td028ttec1");
> +MODULE_ALIAS("spi:tpo,td028ttec1");
Doesn't this mean that the module won't load if you have old bindings?
Can't we have two module aliases?
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
[PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
The vendor name was "toppoly" but other panels and the vendor list
have defined it as "tpo". So let's fix it in driver and bindings.
Signed-off-by: H. Nikolaus Schaller
---
.../display/panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt} | 4 ++--
drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c | 4 ++--
drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c | 4 +++-
3 files changed, 7 insertions(+), 5 deletions(-)
rename Documentation/devicetree/bindings/display/panel/{toppoly,td028ttec1.txt
=> tpo,td028ttec1.txt} (84%)
diff --git
a/Documentation/devicetree/bindings/display/panel/toppoly,td028ttec1.txt
b/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt
similarity index 84%
rename from
Documentation/devicetree/bindings/display/panel/toppoly,td028ttec1.txt
rename to Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt
index 7175dc3740ac..ed34253d9fb1 100644
--- a/Documentation/devicetree/bindings/display/panel/toppoly,td028ttec1.txt
+++ b/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt
@@ -2,7 +2,7 @@ Toppoly TD028TTEC1 Panel
Required properties:
-- compatible: "toppoly,td028ttec1"
+- compatible: "tpo,td028ttec1"
Optional properties:
- label: a symbolic name for the panel
@@ -14,7 +14,7 @@ Example
---
lcd-panel: td028ttec1@0 {
- compatible = "toppoly,td028ttec1";
+ compatible = "tpo,td028ttec1";
reg = <0>;
spi-max-frequency = <10>;
spi-cpol;
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
index 0a38a0e8c925..2dab491478c2 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
@@ -452,7 +452,7 @@ static int td028ttec1_panel_remove(struct spi_device *spi)
}
static const struct of_device_id td028ttec1_of_match[] = {
- { .compatible = "omapdss,toppoly,td028ttec1", },
+ { .compatible = "omapdss,tpo,td028ttec1", },
{},
};
@@ -471,7 +471,7 @@ static struct spi_driver td028ttec1_spi_driver = {
module_spi_driver(td028ttec1_spi_driver);
-MODULE_ALIAS("spi:toppoly,td028ttec1");
+MODULE_ALIAS("spi:tpo,td028ttec1");
MODULE_AUTHOR("H. Nikolaus Schaller ");
MODULE_DESCRIPTION("Toppoly TD028TTEC1 panel driver");
MODULE_LICENSE("GPL");
diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c
b/drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c
index 57e9e146ff74..b67b324e9919 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c
@@ -455,6 +455,8 @@ static int td028ttec1_panel_remove(struct spi_device *spi)
}
static const struct of_device_id td028ttec1_of_match[] = {
+ { .compatible = "omapdss,tpo,td028ttec1", },
+ /* keep to not break older DTB */
{ .compatible = "omapdss,toppoly,td028ttec1", },
{},
};
@@ -474,7 +476,7 @@ static struct spi_driver td028ttec1_spi_driver = {
module_spi_driver(td028ttec1_spi_driver);
-MODULE_ALIAS("spi:toppoly,td028ttec1");
+MODULE_ALIAS("spi:tpo,td028ttec1");
MODULE_AUTHOR("H. Nikolaus Schaller ");
MODULE_DESCRIPTION("Toppoly TD028TTEC1 panel driver");
MODULE_LICENSE("GPL");
--
2.12.2
