linux-kernel - Re: [PATCH 1/3] serial/imx: add device tree support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BANLkTimgqmJDsAc+6aboszK33aBO2XiC6Q@mail.gmail.com>
Date:	Wed, 22 Jun 2011 21:35:19 -0600
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Shawn Guo <shawn.guo@...escale.com>
Cc:	patches@...aro.org, netdev@...r.kernel.org,
	devicetree-discuss@...ts.ozlabs.org,
	Jason Liu <jason.hui@...aro.org>, linux-kernel@...r.kernel.org,
	Jeremy Kerr <jeremy.kerr@...onical.com>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	linux-arm-kernel@...ts.infradead.org,
	David Gibson <david@...son.dropbear.id.au>
Subject: Re: [PATCH 1/3] serial/imx: add device tree support

On Wed, Jun 22, 2011 at 6:12 PM, Shawn Guo <shawn.guo@...escale.com> wrote:
> On Wed, Jun 22, 2011 at 09:52:11AM -0600, Grant Likely wrote:
> [...]
>>
>> >> > -       line = of_get_property(node, "id", NULL);
>> >> > -       if (!line)
>> >> > +       line = of_get_device_index(node, "serial");
>> >> > +       if (IS_ERR_VALUE(line))
>> >> >                return -ENODEV;
>> >>
>> >> Personally, it an alias isn't present, then I'd dynamically assign a port id.
>> >>
>> > We probably can not.  The driver works with being told the correct
>> > port id which is defined by soc.  Instead of dynamically assigning
>> > a port id, we have to tell the driver the exact hardware port id of
>> > the device that is being probed.
>>
>> Are you sure?  It doesn't look like the driver behaviour uses id for
>> anything other than an index into the statically allocated serial port
>> instance table.  I don't see any change of behaviour based on the port
>> number anywhere.
>>
> Sorry, I did not make this clear.  In serial_imx_probe(), the port
> gets created and then saved as below.
>
>        imx_ports[sport->port.line] = sport;
>
> While in imx_console_setup(), it addresses the port as following.
>
>        sport = imx_ports[co->index];
>
> When users specify their console as ttymxc0, they mean they are
> using the first i.mx uart hardware port, in turn ttymxc1 for the
> second port ...
>
> That said, imx_port[0] has to be the first hardware port, imx_port[1]
> has to be the second one ...  That's why port id sport->port.line
> can not be dynamically assigned, otherwise console may not work.

My point still stands.  If there is no number specifically assigned to
a device, then numbering should be dynamic.  That's standard behaviour
for Linux device drivers, and I get nervous every time I see a driver
that make an assumption to the contrary, especially when there is no
good reason for it.  Besides, you still can ensure the console is
reliable by having an alias for the console device.

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/