On 11/16/2017 10:10 AM, H. Nikolaus Schaller wrote:

Hi Andrew,

quoted

Am 16.11.2017 um 16:53 schrieb Andrew F. Davis [off-list ref]:

On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:

quoted

quoted

Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen [off-list ref]:

On 16/11/17 10:50, H. Nikolaus Schaller wrote:

quoted

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 <redacted>
---

quoted

-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.

quoted

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.

quoted

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

quoted

quoted

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 majordomo@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html