| Message ID | 1390208555-27770-2-git-send-email-tushar.behera@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > uart_register_driver call binds the driver to a specific device > node through tty_register_driver call. This should typically happen > during device probe call. > > In a multiplatform scenario, it is possible that multiple serial > drivers are part of the kernel. Currently the driver registration fails > if multiple serial drivers with same default major/minor numbers are > included in the kernel. > > A typical case is observed with amba-pl011 and samsung-uart drivers. The samsung-uart driver is at fault here - the major/minor numbers were officially registered to amba-pl011. Samsung needs to be fixed properly.
On 20 January 2014 15:35, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: >> uart_register_driver call binds the driver to a specific device >> node through tty_register_driver call. This should typically happen >> during device probe call. >> >> In a multiplatform scenario, it is possible that multiple serial >> drivers are part of the kernel. Currently the driver registration fails >> if multiple serial drivers with same default major/minor numbers are >> included in the kernel. >> >> A typical case is observed with amba-pl011 and samsung-uart drivers. > > The samsung-uart driver is at fault here - the major/minor numbers were > officially registered to amba-pl011. Samsung needs to be fixed properly. > I had earlier submitted a patch [1] to remove the hard coded major/minor number for Samsung UART driver, but that was rejected because of userspace breakage. Without this patch, Samsung UART driver can't bind to the hard-coded device node. Changing the default major/minor will also not help to fix the objections raised in [1]. Would you please suggest a way forward? [1] https://lkml.org/lkml/2013/12/27/2
On Mon, Jan 20, 2014 at 05:23:21PM +0530, Tushar Behera wrote: > On 20 January 2014 15:35, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > >> uart_register_driver call binds the driver to a specific device > >> node through tty_register_driver call. This should typically happen > >> during device probe call. > >> > >> In a multiplatform scenario, it is possible that multiple serial > >> drivers are part of the kernel. Currently the driver registration fails > >> if multiple serial drivers with same default major/minor numbers are > >> included in the kernel. > >> > >> A typical case is observed with amba-pl011 and samsung-uart drivers. > > > > The samsung-uart driver is at fault here - the major/minor numbers were > > officially registered to amba-pl011. Samsung needs to be fixed properly. > > > > I had earlier submitted a patch [1] to remove the hard coded > major/minor number for Samsung UART driver, but that was rejected > because of userspace breakage. Without this patch, Samsung UART driver > can't bind to the hard-coded device node. Changing the default > major/minor will also not help to fix the objections raised in [1]. > > Would you please suggest a way forward? I still assert that it's for Samsung people to sort out their abuse of the major/minor numbers - we have a procedure in place to allocate these, I followed it for the AMBA PL011 driver, they didn't. Their problem - and they have to suffer with the consequences of that bad decision on their part, which is that they have to deal with the inevitable breakage caused by having to renumber. > [1] https://lkml.org/lkml/2013/12/27/2 lkml.org is dead at the moment - tried from two different ISPs in the UK, it's nameservers are unreachable, and its https port refuses connections.
On Mon, Jan 20, 2014 at 10:05:30AM +0000, Russell King - ARM Linux wrote: > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > > uart_register_driver call binds the driver to a specific device > > node through tty_register_driver call. This should typically happen > > during device probe call. > > > > In a multiplatform scenario, it is possible that multiple serial > > drivers are part of the kernel. Currently the driver registration fails > > if multiple serial drivers with same default major/minor numbers are > > included in the kernel. > > > > A typical case is observed with amba-pl011 and samsung-uart drivers. > > The samsung-uart driver is at fault here - the major/minor numbers were > officially registered to amba-pl011. Samsung needs to be fixed properly. I agree, the Samsung driver is "broken" here, but that's no reason why these two drivers can't register with the tty layer _after_ the hardware is detected, and not before. That saves resources on systems that build the drivers in, yet do not have the hardware present, which is always a good thing. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 20, 2014 at 01:16:01PM -0800, Greg KH wrote: > On Mon, Jan 20, 2014 at 10:05:30AM +0000, Russell King - ARM Linux wrote: > > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > > > uart_register_driver call binds the driver to a specific device > > > node through tty_register_driver call. This should typically happen > > > during device probe call. > > > > > > In a multiplatform scenario, it is possible that multiple serial > > > drivers are part of the kernel. Currently the driver registration fails > > > if multiple serial drivers with same default major/minor numbers are > > > included in the kernel. > > > > > > A typical case is observed with amba-pl011 and samsung-uart drivers. > > > > The samsung-uart driver is at fault here - the major/minor numbers were > > officially registered to amba-pl011. Samsung needs to be fixed properly. > > I agree, the Samsung driver is "broken" here, but that's no reason why > these two drivers can't register with the tty layer _after_ the hardware > is detected, and not before. > > That saves resources on systems that build the drivers in, yet do not > have the hardware present, which is always a good thing. Great, so what you're saying is that we need to wait until the first device calls into the probe function. What about removal... how does a driver know when it's last device has been removed to de-register that? I guess it needs the driver model to provide some way to know when a driver is completely unbound - but isn't that racy?
> I had earlier submitted a patch [1] to remove the hard coded > major/minor number for Samsung UART driver, but that was rejected > because of userspace breakage. Without this patch, Samsung UART driver > can't bind to the hard-coded device node. Changing the default > major/minor will also not help to fix the objections raised in [1]. > > Would you please suggest a way forward? > > [1] https://lkml.org/lkml/2013/12/27/2 The dynamic major/minor is the right patch. If the userspace breaks then the userspace was broken, but I see no evidence in the discussion that the userspace broke. 204,64 belongs to Altix so neither of the clashing drivers should use it. We had a table to stop messes like this, and we have dynamic numbering to stop the table being needed for the future 50 = /dev/ttyIOC0 Altix serial card ... 81 = /dev/ttyIOC31 Altix serial card Thats what the list says. Samsung should have followed the rules, they didn't so they get to pick up the pieces. The Amba driver wants moving as well. It's easy. If you want something to be ABI then make sure you get it upstream first, if not you get to own all the pain down the line. Hacking up the core code to paper over this crap is not on. We've been insisting firmly and robustly for years that people don't keep borrowing names and numbers they are not allocated, and use dynamic values whenever possible. If the hardware isn't present then the driver shouldn't even register with the tty layer in the first place so it doesn't make any resource differeneces either for properly written code. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 20, 2014 at 09:32:06PM +0000, Russell King - ARM Linux wrote: > On Mon, Jan 20, 2014 at 01:16:01PM -0800, Greg KH wrote: > > On Mon, Jan 20, 2014 at 10:05:30AM +0000, Russell King - ARM Linux wrote: > > > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > > > > uart_register_driver call binds the driver to a specific device > > > > node through tty_register_driver call. This should typically happen > > > > during device probe call. > > > > > > > > In a multiplatform scenario, it is possible that multiple serial > > > > drivers are part of the kernel. Currently the driver registration fails > > > > if multiple serial drivers with same default major/minor numbers are > > > > included in the kernel. > > > > > > > > A typical case is observed with amba-pl011 and samsung-uart drivers. > > > > > > The samsung-uart driver is at fault here - the major/minor numbers were > > > officially registered to amba-pl011. Samsung needs to be fixed properly. > > > > I agree, the Samsung driver is "broken" here, but that's no reason why > > these two drivers can't register with the tty layer _after_ the hardware > > is detected, and not before. > > > > That saves resources on systems that build the drivers in, yet do not > > have the hardware present, which is always a good thing. > > Great, so what you're saying is that we need to wait until the first > device calls into the probe function. What about removal... how does > a driver know when it's last device has been removed to de-register > that? The "bus" that the device is on handles that, right? > I guess it needs the driver model to provide some way to know when a > driver is completely unbound - but isn't that racy? How is it racy? That's how the driver model works... thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 20, 2014 at 09:43:05PM +0000, Alan Cox wrote: > The dynamic major/minor is the right patch. If the userspace breaks then > the userspace was broken, but I see no evidence in the discussion that > the userspace broke. The userspace breakage is that if someone has a static /dev that doesn't handle any dynamic devices then renumbering the device will cause that static /dev to stop matching the kernel. > Thats what the list says. Samsung should have followed the rules, they > didn't so they get to pick up the pieces. The Amba driver wants moving as > well. It's easy. If you want something to be ABI then make sure you get > it upstream first, if not you get to own all the pain down the line. This stuff is all upstream already, a quick check suggests both drivers predate git - it's been noticed because the ARM multiplatform work has caused people to try booting kernels with both built in. > If the hardware isn't present then the driver shouldn't even register > with the tty layer in the first place so it doesn't make any resource > differeneces either for properly written code. Right, that's not the idiom that has been followed by any of serial drivers though so needs fixing too.
On Mon, Jan 20, 2014 at 03:11:41PM -0800, Greg KH wrote: > On Mon, Jan 20, 2014 at 09:32:06PM +0000, Russell King - ARM Linux wrote: > > On Mon, Jan 20, 2014 at 01:16:01PM -0800, Greg KH wrote: > > > On Mon, Jan 20, 2014 at 10:05:30AM +0000, Russell King - ARM Linux wrote: > > > > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > > > > > uart_register_driver call binds the driver to a specific device > > > > > node through tty_register_driver call. This should typically happen > > > > > during device probe call. > > > > > > > > > > In a multiplatform scenario, it is possible that multiple serial > > > > > drivers are part of the kernel. Currently the driver registration fails > > > > > if multiple serial drivers with same default major/minor numbers are > > > > > included in the kernel. > > > > > > > > > > A typical case is observed with amba-pl011 and samsung-uart drivers. > > > > > > > > The samsung-uart driver is at fault here - the major/minor numbers were > > > > officially registered to amba-pl011. Samsung needs to be fixed properly. > > > > > > I agree, the Samsung driver is "broken" here, but that's no reason why > > > these two drivers can't register with the tty layer _after_ the hardware > > > is detected, and not before. > > > > > > That saves resources on systems that build the drivers in, yet do not > > > have the hardware present, which is always a good thing. > > > > Great, so what you're saying is that we need to wait until the first > > device calls into the probe function. What about removal... how does > > a driver know when it's last device has been removed to de-register > > that? > > The "bus" that the device is on handles that, right? > > > I guess it needs the driver model to provide some way to know when a > > driver is completely unbound - but isn't that racy? > > How is it racy? That's how the driver model works... Think about what happens when the last device unregisters, but a new device comes along and is probed. I don't believe the driver model has any locking to prevent a drivers ->probe function running concurrently with it's ->remove function for two (or more) devices. The locking against this is done on a per-device basis, not a per-driver basis.
On Mon, Jan 20, 2014 at 11:14:57PM +0000, Mark Brown wrote: > On Mon, Jan 20, 2014 at 09:43:05PM +0000, Alan Cox wrote: > > If the hardware isn't present then the driver shouldn't even register > > with the tty layer in the first place so it doesn't make any resource > > differeneces either for properly written code. > > Right, that's not the idiom that has been followed by any of serial > drivers though so needs fixing too. It's not followed by serial drivers because it gets f*scking complicated to do it that way. In order to do it that way, what we need to do is: 1. On the first device probe, register the UART driver. 2. On subsequent device probes, don't register the UART driver because it's already registered. 3. When devices are removed, do nothing until the last device. 4. When the last device is removed, unregister the UART driver. The first bit is easy... but we need to add locks to every serial driver to prevent two probes operating concurrently...
> The first bit is easy... but we need to add locks to every serial > driver to prevent two probes operating concurrently... The bus probe should already be serializing surely ? -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 20 Jan 2014 23:14:57 +0000 Mark Brown <broonie@kernel.org> wrote: > On Mon, Jan 20, 2014 at 09:43:05PM +0000, Alan Cox wrote: > > > The dynamic major/minor is the right patch. If the userspace breaks then > > the userspace was broken, but I see no evidence in the discussion that > > the userspace broke. > > The userspace breakage is that if someone has a static /dev that doesn't > handle any dynamic devices then renumbering the device will cause that > static /dev to stop matching the kernel. Diddums and you've only provided theoretical cases not real world ones. They should have followed proper practice and reserved their minors. The device number belongs to the Altix. The driver should just move. > > Thats what the list says. Samsung should have followed the rules, they > > didn't so they get to pick up the pieces. The Amba driver wants moving as > > well. It's easy. If you want something to be ABI then make sure you get > > it upstream first, if not you get to own all the pain down the line. > > This stuff is all upstream already, a quick check suggests both drivers > predate git - it's been noticed because the ARM multiplatform work has > caused people to try booting kernels with both built in. So ARM people didn't follow the policy on allocating device minors even within their own community and got burned by it. That's despite having previously been burned by abusing the ttyS0 8250 major/minor in the same way, for the same purposes, and creating the same mess. {facepalm...} > > If the hardware isn't present then the driver shouldn't even register > > with the tty layer in the first place so it doesn't make any resource > > differeneces either for properly written code. > > Right, that's not the idiom that has been followed by any of serial > drivers though so needs fixing too. Actally some drivers do get this right but not many. ehv-bc for example does. But yes I agree about the idiom, but a definite NAK to any attempts to plaster over this grand screwup by crapping in the tty core. Your turd, deal with it locally in the ARM code if you can't apply common sense and just go dynamic. And please, after screwing this up twice - *learn* from the mess. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 20, 2014 at 11:16:03PM +0000, Russell King - ARM Linux wrote: > On Mon, Jan 20, 2014 at 03:11:41PM -0800, Greg KH wrote: > > On Mon, Jan 20, 2014 at 09:32:06PM +0000, Russell King - ARM Linux wrote: > > > On Mon, Jan 20, 2014 at 01:16:01PM -0800, Greg KH wrote: > > > > On Mon, Jan 20, 2014 at 10:05:30AM +0000, Russell King - ARM Linux wrote: > > > > > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > > > > > > uart_register_driver call binds the driver to a specific device > > > > > > node through tty_register_driver call. This should typically happen > > > > > > during device probe call. > > > > > > > > > > > > In a multiplatform scenario, it is possible that multiple serial > > > > > > drivers are part of the kernel. Currently the driver registration fails > > > > > > if multiple serial drivers with same default major/minor numbers are > > > > > > included in the kernel. > > > > > > > > > > > > A typical case is observed with amba-pl011 and samsung-uart drivers. > > > > > > > > > > The samsung-uart driver is at fault here - the major/minor numbers were > > > > > officially registered to amba-pl011. Samsung needs to be fixed properly. > > > > > > > > I agree, the Samsung driver is "broken" here, but that's no reason why > > > > these two drivers can't register with the tty layer _after_ the hardware > > > > is detected, and not before. > > > > > > > > That saves resources on systems that build the drivers in, yet do not > > > > have the hardware present, which is always a good thing. > > > > > > Great, so what you're saying is that we need to wait until the first > > > device calls into the probe function. What about removal... how does > > > a driver know when it's last device has been removed to de-register > > > that? > > > > The "bus" that the device is on handles that, right? > > > > > I guess it needs the driver model to provide some way to know when a > > > driver is completely unbound - but isn't that racy? > > > > How is it racy? That's how the driver model works... > > Think about what happens when the last device unregisters, but a new > device comes along and is probed. > > I don't believe the driver model has any locking to prevent a drivers > ->probe function running concurrently with it's ->remove function for > two (or more) devices. The bus prevents this from happening. > The locking against this is done on a per-device basis, not a per-driver > basis. No, on a per-bus basis. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 20, 2014 at 11:35:41PM +0000, Alan Cox wrote: > > The first bit is easy... but we need to add locks to every serial > > driver to prevent two probes operating concurrently... > > The bus probe should already be serializing surely ? Yes, it better be, otherwise that bus is badly broken. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 20, 2014 at 03:51:28PM -0800, Greg KH wrote: > On Mon, Jan 20, 2014 at 11:16:03PM +0000, Russell King - ARM Linux wrote: > > I don't believe the driver model has any locking to prevent a drivers > > ->probe function running concurrently with it's ->remove function for > > two (or more) devices. > > The bus prevents this from happening. > > > The locking against this is done on a per-device basis, not a per-driver > > basis. > > No, on a per-bus basis. I don't see it. Let's start from driver_register(). This takes no locks and calls bus_add_driver(). This also takes no locks and calls driver_attach(). This walks the list of devices calling __driver_attach() for each. __driver_attach() tries to match the device against the driver, locks the parent device if one exists, and the device which is about to be probed. It then calls driver_probe_device(). driver_probe_device() inserts a runtime barrier and calls really_probe(). really_probe() ultimately calls either the bus ->probe method or the driver ->probe method. At no point in that sequence do I see anything which does any locking on a per-driver basis. Let's look at device_add(). device_add() calls bus_probe_device(), which then calls device_attach(). device_attach() takes the device's lock, and walks the list of drivers calling __device_attach() on each driver. This then calls down into driver_probe_device(), and the path is the same as the above. I don't see any per-driver locking here either. I've checked the klist stuff, don't see anything there. Ditto for bus_for_each_drv(). If you think there's a per-driver lock that's held over probes or removes, please point it out. I'm fairly certain that there isn't, because we have to be able to deal with recursive probes (yes, we've had to deal with those in the past.)
On Mon, Jan 20, 2014 at 11:47:34PM +0000, Alan Cox wrote: > But yes I agree about the idiom, but a definite NAK to any attempts to > plaster over this grand screwup by crapping in the tty core. Your turd, > deal with it locally in the ARM code if you can't apply common sense and > just go dynamic. I believe at the time there was no one maintaining the device list to _do_ that allocation - AMBA PL011 came along in 2005 after (I believe) hpa stopped looking after that list. So, please tell me how a number could be allocated properly without the device numbers list being maintained? I've no problem with going dynamic, and I suggest that you get a sense of perspective rather than just spouting rubbish from on high.
On Tue, Jan 21, 2014 at 12:07:06AM +0000, Russell King - ARM Linux wrote: > On Mon, Jan 20, 2014 at 03:51:28PM -0800, Greg KH wrote: > > On Mon, Jan 20, 2014 at 11:16:03PM +0000, Russell King - ARM Linux wrote: > > > I don't believe the driver model has any locking to prevent a drivers > > > ->probe function running concurrently with it's ->remove function for > > > two (or more) devices. > > > > The bus prevents this from happening. > > > > > The locking against this is done on a per-device basis, not a per-driver > > > basis. > > > > No, on a per-bus basis. > > I don't see it. > > Let's start from driver_register(). Which happens from module probing, which is single-threaded, right? Or from module_init callbacks, which is single-threaded. Normally, busses never add devices (which is what drivers bind to), except in a single-at-a-time fashion, unless they really know what they are doing (i.e. PCI had multi-threaded device probing for a while, don't remember if it still does...) > This takes no locks and calls bus_add_driver(). > This also takes no locks and calls driver_attach(). > This walks the list of devices calling __driver_attach() for each. > __driver_attach() tries to match the device against the driver, > locks the parent device if one exists, and the device which is about > to be probed. It then calls driver_probe_device(). > driver_probe_device() inserts a runtime barrier and calls really_probe(). > really_probe() ultimately calls either the bus ->probe method or the > driver ->probe method. > > At no point in that sequence do I see anything which does any locking > on a per-driver basis. Let's look at device_add(). > > device_add() calls bus_probe_device(), which then calls device_attach(). > device_attach() takes the device's lock, and walks the list of drivers > calling __device_attach() on each driver. This then calls down into > driver_probe_device(), and the path is the same as the above. > > I don't see any per-driver locking here either. > > I've checked the klist stuff, don't see anything there. Ditto for > bus_for_each_drv(). > > If you think there's a per-driver lock that's held over probes or removes, > please point it out. I'm fairly certain that there isn't, because we have > to be able to deal with recursive probes (yes, we've had to deal with > those in the past.) Hm, you are right, I think that's why we had to remove the locks. The klist stuff handles us getting the needed locks for managing our internal lists of devices and drivers, and those should be fine. So, let's go back to your original worry, what are you concerned about? A device being removed while probe() is called? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 20, 2014 at 04:26:23PM -0800, Greg KH wrote: > On Tue, Jan 21, 2014 at 12:07:06AM +0000, Russell King - ARM Linux wrote: > > On Mon, Jan 20, 2014 at 03:51:28PM -0800, Greg KH wrote: > > > On Mon, Jan 20, 2014 at 11:16:03PM +0000, Russell King - ARM Linux wrote: > > > > I don't believe the driver model has any locking to prevent a drivers > > > > ->probe function running concurrently with it's ->remove function for > > > > two (or more) devices. > > > > > > The bus prevents this from happening. > > > > > > > The locking against this is done on a per-device basis, not a per-driver > > > > basis. > > > > > > No, on a per-bus basis. > > > > I don't see it. > > > > Let's start from driver_register(). > > Which happens from module probing, which is single-threaded, right? Yes, to _some_ extent - the driver is added to the bus list of drivers before existing drivers are probed, so it's always worth bearing in mind that if a new device comes along, it's possible for that device to be offered to even a driver which hasn't finished returning from its module_init(). > > If you think there's a per-driver lock that's held over probes or removes, > > please point it out. I'm fairly certain that there isn't, because we have > > to be able to deal with recursive probes (yes, we've had to deal with > > those in the past.) > > Hm, you are right, I think that's why we had to remove the locks. The > klist stuff handles us getting the needed locks for managing our > internal lists of devices and drivers, and those should be fine. > > So, let's go back to your original worry, what are you concerned about? > A device being removed while probe() is called? My concern is that we're turning something which should be simple into something unnecessarily complex. By that, I mean something along the lines of: static DEFINE_MUTEX(foo_mutex); static unsigned foo_devices; static int foo_probe(struct platform_device *pdev) { int ret; mutex_lock(&foo_mutex); if (foo_devices++ == 0) uart_register_driver(&driver); ret = foo_really_probe_device(pdev); if (ret) { if (--foo_devices == 0) uart_unregister_driver(&driver); } mutex_unlock(&foo_mutex); return ret; } static int foo_remove(struct platform_device *pdev) { mutex_lock(&foo_mutex); foo_really_remove(pdev); if (--foo_devices == 0) uart_unregister_driver(&driver); mutex_unlock(&foo_mutex); return 0; } in every single serial driver we have... Wouldn't it just be better to fix the major/minor number problem rather than have to add all that code repetitively to all those drivers?
On Tue, 21 Jan 2014 00:16:57 +0000 Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: [I did post a reply to this while on my phone but it got rejected] > On Mon, Jan 20, 2014 at 11:47:34PM +0000, Alan Cox wrote: > > But yes I agree about the idiom, but a definite NAK to any attempts to > > plaster over this grand screwup by crapping in the tty core. Your turd, > > deal with it locally in the ARM code if you can't apply common sense and > > just go dynamic. > > I believe at the time there was no one maintaining the device list to > _do_ that allocation - AMBA PL011 came along in 2005 after (I believe) > hpa stopped looking after that list. git log Documentation/devices.txt > So, please tell me how a number could be allocated properly without the > device numbers list being maintained? It was being maintained > I've no problem with going dynamic, and I suggest that you get a sense > of perspective rather than just spouting rubbish from on high. I suggest you take a harder look at the actual history rather than your revisionist one and then apologise. The "simple" way to sort this out is to go dynamic as first proposed. The more complicated way *IFF* Ben can show an actual systems that break is for the ARM folks to bury a "Use ancient static device mapping" KConfig entry into the arch Kconfig - which can then go away after a while. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > So, let's go back to your original worry, what are you concerned about? > > A device being removed while probe() is called? > > My concern is that we're turning something which should be simple into > something unnecessarily complex. By that, I mean something along the > lines of: Or in fact more complex in other cases because your remove may well be refcounted so the stuff may not be going away in the foo_remove() path. > > static DEFINE_MUTEX(foo_mutex); > static unsigned foo_devices; > > static int foo_probe(struct platform_device *pdev) > { > int ret; > > mutex_lock(&foo_mutex); > if (foo_devices++ == 0) > uart_register_driver(&driver); > > ret = foo_really_probe_device(pdev); We have atomic_inc_and_test and atomic_dec_and_test so it's fractionally less ugly. > in every single serial driver we have... Wouldn't it just be better to > fix the major/minor number problem rather than have to add all that code > repetitively to all those drivers? Quite. Although for some drivers I suspect what is actually missing when built in is module_init() { if (not_the_right_platform()) return -ENOGOOD; } Going dynamic is the right fix though. Changing how the driver registration work is a different (and quite independent) problem. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 21, 2014 at 09:25:31AM +0000, One Thousand Gnomes wrote: > > static DEFINE_MUTEX(foo_mutex); > > static unsigned foo_devices; > > > > static int foo_probe(struct platform_device *pdev) > > { > > int ret; > > > > mutex_lock(&foo_mutex); > > if (foo_devices++ == 0) > > uart_register_driver(&driver); > > > > ret = foo_really_probe_device(pdev); > > We have atomic_inc_and_test and atomic_dec_and_test so it's > fractionally less ugly. How do atomics help here? If we do this as: if (atomic_inc_and_test(&foo_atomic)) uart_register_driver(&driver); Then let's think about what can happen: CPU0 CPU1 foo_probe atomic_inc_and_test() == true uart_register_driver foo_probe atomic_inc_and_test() really_probe_foo() *bang*
On Tue, Jan 21, 2014 at 09:03:40AM +0000, Alan Cox wrote: > On Tue, 21 Jan 2014 00:16:57 +0000 > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > [I did post a reply to this while on my phone but it got rejected] > > > On Mon, Jan 20, 2014 at 11:47:34PM +0000, Alan Cox wrote: > > > But yes I agree about the idiom, but a definite NAK to any attempts to > > > plaster over this grand screwup by crapping in the tty core. Your turd, > > > deal with it locally in the ARM code if you can't apply common sense and > > > just go dynamic. > > > > I believe at the time there was no one maintaining the device list to > > _do_ that allocation - AMBA PL011 came along in 2005 after (I believe) > > hpa stopped looking after that list. > > git log Documentation/devices.txt Hmm. Given that git history starts at 2.6.12-rc2, and this was introduced in 2.5 kernels, there's no point me looking, because I know the history I have access to does not go back to that time. And I know for certain that the devices list *did* fall out of maintainership for a while.
On Mon, Jan 20, 2014 at 11:47:34PM +0000, Alan Cox wrote: > Mark Brown <broonie@kernel.org> wrote: > > On Mon, Jan 20, 2014 at 09:43:05PM +0000, Alan Cox wrote: > > The userspace breakage is that if someone has a static /dev that doesn't > > handle any dynamic devices then renumbering the device will cause that > > static /dev to stop matching the kernel. > Diddums and you've only provided theoretical cases not real world ones. I'm fairly sure I have some older boards on my desk with static /dev, I'd need to go and check which is rather more effort than I'm interested in putting in for point scoring. > They should have followed proper practice and reserved their minors. The > device number belongs to the Altix. The driver should just move. They should have done that about a decade ago when this stuff was introduced, yes. At some point one has to accept and deal with the world as it is. > But yes I agree about the idiom, but a definite NAK to any attempts to > plaster over this grand screwup by crapping in the tty core. Your turd, > deal with it locally in the ARM code if you can't apply common sense and > just go dynamic. Local bodges being the first two patches Greg knocked back of course... In any case, if we can convince people to make the subsystem better and coincidentally also stop problems occuring on their systems that does seem like a win overall. > And please, after screwing this up twice - *learn* from the mess. One of the things we've been trying to do is to ensure that the all the driver code that isn't core architecture support (not just for ARM, for everything) is going into subsystems so we're getting good review from people who actually know the subsystems and aren't just saying things are random platform stuff or whatever that we don't care about. That has been a source of problems all over, we're trying to avoid it.
On Tue, Jan 21, 2014 at 04:59:35PM +0000, Mark Brown wrote: > On Mon, Jan 20, 2014 at 11:47:34PM +0000, Alan Cox wrote: > > They should have followed proper practice and reserved their minors. The > > device number belongs to the Altix. The driver should just move. > > They should have done that about a decade ago when this stuff was > introduced, yes. At some point one has to accept and deal with the > world as it is. Let's look at the facts, shall we? We have several drivers which I was involved in, which include the original AMBA PL010 driver, the SA11x0 serial driver, and the Footbridge driver. Both of those had been around for some time in earlier kernels. I remember there being a discussion about the major/minor numbers, and the conclusion which came out of that was to have this "low density serial ports" major, and allocate minor numbers from that. So I applied for minor numbers for these drivers, and HPA at the time granted them. Here's an exmaple: | Russell King wrote: | > | > Hi hpa, | > | > Did you get my previous mail about this device allocation? If not, here | > is the request (although its hard to put it together without an up to | > date devices.txt list): | > | > Major: 204 | > Minor: 16 | > Number of minors: 16 | > Non-devfs name: ttyAM0 to ttyAM15 | > Description: ARM "AMBA" serial ports | > | > Major: 205 | > Minor: 16 | > Number of minors: 16 | > Non-devfs name: cuaam to cuaam15 | > Description: Callout device for ttyAM0 to ttyAM15 | > | | Approved as written. Others did the same (previously) for the StrongARM driver (which was Nicolas). During the 2.3/2.4 era, the clps711x serial driver was added, and that re-used the above allocations since it was believed at the time that there would be no overlap (they'd only appear on SoCs and you would never have one of each type on each SoC). So, the clps711x serial driver never had an offical allocation at that time. Later on, it transitioned to using its own minors (which now conflicts). Exactly when that happened, I'm not sure - sometime between Jan and July 2002. However, the clps711x driver went in as a side effect of the need to replace the old serial.c driver with something better - and it was decided that my serial_core and serial rewrite was the desired way forward. So that stuff got sucked in with no official allocation - that was an oversight on my part. At some point after that, the amba PL011 driver was added - I can't pin that down, because the history from the 2.5/early 2.6 is lost to me (and I can't sanely get access to the Linux history git... not with the state of my DSL.) Looking at the patch archive I have, it may have been around patch-2.6.0-test2 time when amba-pl011 came into being, which places that around 5th August 2003, but I can't see when it went into mainline since I stopped producing patches in December 2003, but we can be certain that it was after 2003. Now, I'm certain of my facts here that LANANA stopped being useful around this time - especially so now that I've found an email from 2004: | Date: Wed, 23 Jun 2004 10:07:54 -0500 | From: Erik Jacobson <erikj@subway.americas.sgi.com> | Subject: Re: [PATCH 2.6] Altix serial driver | Message-ID: <Pine.SGI.4.53.0406231005110.280312@subway.americas.sgi.com> | | LANANA has been unresponsive and their site states they aren't accepting | 2.6 submissions. So here we have evidence that I'm not the only one who stumbled across LANANA becoming a problem - and it becoming impossible to have additional numbers allocated in that range. Here's more evidence, from later on, which appears to show when LANANA resumed: | From: David Woodhouse <dwmw2@infradead.org> | To: device@lanana.org | Cc: benh@kernel.crashing.org, rmk+serial@arm.linux.org.uk | Date: Fri, 01 Dec 2006 15:40:23 +0000 | Message-Id: <1164987623.12649.36.camel@pmac.infradead.org> | | Please assign four minor numbers from major 204, for the pmac_zilog | driver. | | Thanks. To which LANANA never responded, so it was followed up: | From: David Woodhouse <dwmw2@infradead.org> | To: device@lanana.org | Cc: benh@kernel.crashing.org, rmk+serial@arm.linux.org.uk | Date: Fri, 30 Mar 2007 04:11:52 +0100 | Message-Id: <1175224312.3122.113.camel@pmac.infradead.org> | | On Fri, 2006-12-01 at 15:40 +0000, David Woodhouse wrote: | > Please assign four minor numbers from major 204, for the pmac_zilog | > driver. | | Pretty please? And this finally resulted in numbers being allocated again, this time by Torben Mathiasen. Now, I had - and have - absolutely *no* problem with this process... and if it had been available for allocating the ttyAMA numbers properly, I would have definitely used it - for the sole reason that it is the only sane thing to do. But it wasn't available, and something had to be done. Unlike Alan's comments which would have you believe that LANANA has been operating continuously, it seems that the evidence I have backs up my claim, and supports the reason why these drivers never had an official allocation, and it's certainly *not* through lack of common sense or the intention of creating "a turd" - which I take great offence to those accusations. I did the best I could at that time given the circumstances that were prevailing.
> I had earlier submitted a patch [1] to remove the hard coded > major/minor number for Samsung UART driver, but that was rejected > because of userspace breakage. Without this patch, Samsung UART driver > can't bind to the hard-coded device node. Changing the default > major/minor will also not help to fix the objections raised in [1]. > > Would you please suggest a way forward? > > [1] https://lkml.org/lkml/2013/12/27/2 So to go and try and put this to bed properly I would suggest the following way forward. We add CONFIG_LEGACY_STATIC_TTY Some platforms historically used static device nodes for the console devices. Select this if you are building a kernel for an old system which has a static /dev. Note that because some devices historically used incorrect clashing numbering this may prevent you building a single kernel which can be booted on multiple platforms. And then we do .nr = CONFIG_SERIAL_SAMSUNG_UARTS, .cons = S3C24XX_SERIAL_CONSOLE, .dev_name = S3C24XX_SERIAL_NAME, #ifdef CONFIG_LEGACY_STATIC_TTY .major = S3C24XX_SERIAL_MAJOR, .minor = S3C24XX_SERIAL_MINOR, #endif for the afflicted ports (and anyone else who wants to migrate) We can then enable that config option for ARM (and in time for any other architecture that turns out to need/want it). Eventually it can go away (not that its exactly doing any harm if it doesnt). Does that sound a valid way forward for everyone ? Alan -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 23, 2014 at 06:04:23PM +0000, Alan Cox wrote: > We can then enable that config option for ARM (and in time for any other > architecture that turns out to need/want it). Eventually it can go away > (not that its exactly doing any harm if it doesnt). We'd need to leave it user selectable rather than enabling it for ARM, the whole reason this got noticed is that people are trying to build kernels that support a wider range of devices for ARM.
On 23.01.2014 19:40, Mark Brown wrote: > On Thu, Jan 23, 2014 at 06:04:23PM +0000, Alan Cox wrote: > >> We can then enable that config option for ARM (and in time for any other >> architecture that turns out to need/want it). Eventually it can go away >> (not that its exactly doing any harm if it doesnt). > > We'd need to leave it user selectable rather than enabling it for ARM, > the whole reason this got noticed is that people are trying to build > kernels that support a wider range of devices for ARM. What about making it depend on !MULTIPLATFORM and enabled by default? Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 23, 2014 at 07:47:56PM +0100, Tomasz Figa wrote: > On 23.01.2014 19:40, Mark Brown wrote: > >We'd need to leave it user selectable rather than enabling it for ARM, > >the whole reason this got noticed is that people are trying to build > >kernels that support a wider range of devices for ARM. > What about making it depend on !MULTIPLATFORM and enabled by default? That'd work, but if we're doing that then substituting in the dynamic assignment only when we hit a collision seems smoother and more general. Or we could just make the core ignore all hard coded numbers if this is set rather than putting ifdefs in the drivers.
On Thu, 23 Jan 2014 19:36:33 +0000 Mark Brown <broonie@kernel.org> wrote: > On Thu, Jan 23, 2014 at 07:47:56PM +0100, Tomasz Figa wrote: > > On 23.01.2014 19:40, Mark Brown wrote: > > > >We'd need to leave it user selectable rather than enabling it for ARM, > > >the whole reason this got noticed is that people are trying to build > > >kernels that support a wider range of devices for ARM. > > > What about making it depend on !MULTIPLATFORM and enabled by default? > > That'd work, but if we're doing that then substituting in the dynamic > assignment only when we hit a collision seems smoother and more general. > Or we could just make the core ignore all hard coded numbers if this is > set rather than putting ifdefs in the drivers. That strikes me as rather more risky. We can propogate it through the drivers as we are sure it is safe to do so on that platform and encourage driver authors to migrate. Better than a "big bang" and the inevitable fallout. We do want it user selectable, because we want people to leave it off unless they have a need for it - that then becomes a path towards eventually getting rid of them static identifiers for good. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 23, 2014 at 07:51:44PM +0000, Alan Cox wrote: > That strikes me as rather more risky. We can propogate it through the > drivers as we are sure it is safe to do so on that platform and encourage > driver authors to migrate. Better than a "big bang" and the inevitable > fallout. I don't see how we can meaningfully test this on a platform - the kernel would have to be pretty demented to care, it's userspace that cares and that's not really tied to individual serial drivers but is where we mainly need coverage. > We do want it user selectable, because we want people to leave it off > unless they have a need for it - that then becomes a path towards > eventually getting rid of them static identifiers for good. Yup.
On Thu, 23 Jan 2014 20:05:09 +0000 Mark Brown <broonie@kernel.org> wrote: > On Thu, Jan 23, 2014 at 07:51:44PM +0000, Alan Cox wrote: > > > That strikes me as rather more risky. We can propogate it through the > > drivers as we are sure it is safe to do so on that platform and encourage > > driver authors to migrate. Better than a "big bang" and the inevitable > > fallout. > > I don't see how we can meaningfully test this on a platform - the kernel > would have to be pretty demented to care, it's userspace that cares and > that's not really tied to individual serial drivers but is where we > mainly need coverage. Which is why I think we want to enable it gradually and platform by platform as that platform or arch maintainer judges it appropriate given their knowledge of their platform(s). Alan -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 23, 2014 at 09:33:21PM +0000, Alan Cox wrote: > Mark Brown <broonie@kernel.org> wrote: > > I don't see how we can meaningfully test this on a platform - the kernel > > would have to be pretty demented to care, it's userspace that cares and > > that's not really tied to individual serial drivers but is where we > > mainly need coverage. > Which is why I think we want to enable it gradually and platform by > platform as that platform or arch maintainer judges it appropriate given > their knowledge of their platform(s). I don't see how that follows? For the most part architecture maintainers aren't going to be able to say too much about which userspaces are being run on their platforms if the architecture has any kind of widespread use.
On Fri, 24 Jan 2014 12:03:09 +0000 Mark Brown <broonie@kernel.org> wrote: > On Thu, Jan 23, 2014 at 09:33:21PM +0000, Alan Cox wrote: > > Mark Brown <broonie@kernel.org> wrote: > > > > I don't see how we can meaningfully test this on a platform - the kernel > > > would have to be pretty demented to care, it's userspace that cares and > > > that's not really tied to individual serial drivers but is where we > > > mainly need coverage. > > > Which is why I think we want to enable it gradually and platform by > > platform as that platform or arch maintainer judges it appropriate given > > their knowledge of their platform(s). > > I don't see how that follows? For the most part architecture > maintainers aren't going to be able to say too much about which > userspaces are being run on their platforms if the architecture > has any kind of widespread use. For most architectures I suspect that they do have a pretty good idea. ARM is a bit different but I imagine the various platform specific maintainers are the ones who have that knowledge Alan -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 21, 2014 at 12:45:05AM +0000, Alan Cox wrote: > Peter handed it on. Try using git log on Documentation/devices.txt. It > still gets updates. > > Perhaps you'd care to stick to reality and fix the tree instead of trying > to excuse the mess ? Perhaps returning to reality might be advantageous rather than trying to repeat statements which can't have any bearing on this - especially as the git history which you're referring to only goes back to 2.6.12-rc2, and this predates 2.6.12-rc2 by a long shot. > More importantly certain folks need to stop abusing static numbers > allocated properly. Repeating it having made a total hash of it before > is dismal. And if you continue these stupid accusations which have no basis at all, we're going to get into a real big argument, because you are soo _wrong_ on that point. I was always the one arguing /against/ the re-use of existing major/minor numbers. I was the one arguing /against/ Nicolas' patches to make every serial port appear in the 4,64 ttyS namespace.
On Thu 2014-01-23 19:36:33, Mark Brown wrote: > On Thu, Jan 23, 2014 at 07:47:56PM +0100, Tomasz Figa wrote: > > On 23.01.2014 19:40, Mark Brown wrote: > > > >We'd need to leave it user selectable rather than enabling it for ARM, > > >the whole reason this got noticed is that people are trying to build > > >kernels that support a wider range of devices for ARM. > > > What about making it depend on !MULTIPLATFORM and enabled by default? > > That'd work, but if we're doing that then substituting in the dynamic > assignment only when we hit a collision seems smoother and more general. That seems like a mess. I had enough fun debugging "WTF is going on with my serials" as is... Plus... collision can happen at runtime when you insert BT CF card... at that point it is too late to reassign. Pavel
On Sun, 26 Jan 2014 22:09:07 +0100 Pavel Machek <pavel@ucw.cz> wrote: > On Thu 2014-01-23 19:36:33, Mark Brown wrote: > > On Thu, Jan 23, 2014 at 07:47:56PM +0100, Tomasz Figa wrote: > > > On 23.01.2014 19:40, Mark Brown wrote: > > > > > >We'd need to leave it user selectable rather than enabling it for ARM, > > > >the whole reason this got noticed is that people are trying to build > > > >kernels that support a wider range of devices for ARM. > > > > > What about making it depend on !MULTIPLATFORM and enabled by default? > > > > That'd work, but if we're doing that then substituting in the dynamic > > assignment only when we hit a collision seems smoother and more general. > > That seems like a mess. I had enough fun debugging "WTF is > going on with my serials" as is... > > Plus... collision can happen at runtime when you insert > BT CF card... at that point it is too late to reassign. Agreed entirely - you want predictability here, plus making it happen by magic is going to do the nut of anyone trying to figure out why kernel A works on their platform and kernel B doesn't. It needs to be something you choose to turn on as compatibility, like old sysfs files and so forth. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 24, 2014 at 02:38:59PM +0000, Alan Cox wrote: > Mark Brown <broonie@kernel.org> wrote: > > I don't see how that follows? For the most part architecture > > maintainers aren't going to be able to say too much about which > > userspaces are being run on their platforms if the architecture > > has any kind of widespread use. > For most architectures I suspect that they do have a pretty good idea. > ARM is a bit different but I imagine the various platform specific > maintainers are the ones who have that knowledge I'm still not entirely sure how you expect people to answer this unless the answer is something along the lines of "all the systems are on my desk". This definitely does need to be selectable, I'm just not clear that making it depend on the architecture is going to help.
On Sun, 26 Jan 2014, Russell King - ARM Linux wrote: > On Tue, Jan 21, 2014 at 12:45:05AM +0000, Alan Cox wrote: > > Peter handed it on. Try using git log on Documentation/devices.txt. It > > still gets updates. > > > > Perhaps you'd care to stick to reality and fix the tree instead of trying > > to excuse the mess ? > > Perhaps returning to reality might be advantageous rather than trying > to repeat statements which can't have any bearing on this - especially > as the git history which you're referring to only goes back to 2.6.12-rc2, > and this predates 2.6.12-rc2 by a long shot. > > > More importantly certain folks need to stop abusing static numbers > > allocated properly. Repeating it having made a total hash of it before > > is dismal. > > And if you continue these stupid accusations which have no basis at all, > we're going to get into a real big argument, because you are soo _wrong_ > on that point. I was always the one arguing /against/ the re-use of > existing major/minor numbers. I was the one arguing /against/ Nicolas' > patches to make every serial port appear in the 4,64 ttyS namespace. If you remember correctly, that was my attempt at making serial port minor assignment to be dynamic... just like everything else is today. And it seemed to me that you thought this was a good idea. But that was objected by the X86 folks who insisted on always having COM1 == ttyS0 and COM2 == ttyS1, whether or not COM1 was present. Nowdays serial ports have pretty much disappeared from the X86 world. But the problem they've created is still with us. Either everything is dynamic, or everything follows proper minor assignment process. Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Either everything is dynamic, or everything follows proper minor > assignment process. Ultimately everything should probably be dynamic, but trying to get there in one step will simply mean we never get there at all. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jan 26, 2014 at 11:30:00PM -0500, Nicolas Pitre wrote: > On Sun, 26 Jan 2014, Russell King - ARM Linux wrote: > > > On Tue, Jan 21, 2014 at 12:45:05AM +0000, Alan Cox wrote: > > > Peter handed it on. Try using git log on Documentation/devices.txt. It > > > still gets updates. > > > > > > Perhaps you'd care to stick to reality and fix the tree instead of trying > > > to excuse the mess ? > > > > Perhaps returning to reality might be advantageous rather than trying > > to repeat statements which can't have any bearing on this - especially > > as the git history which you're referring to only goes back to 2.6.12-rc2, > > and this predates 2.6.12-rc2 by a long shot. > > > > > More importantly certain folks need to stop abusing static numbers > > > allocated properly. Repeating it having made a total hash of it before > > > is dismal. > > > > And if you continue these stupid accusations which have no basis at all, > > we're going to get into a real big argument, because you are soo _wrong_ > > on that point. I was always the one arguing /against/ the re-use of > > existing major/minor numbers. I was the one arguing /against/ Nicolas' > > patches to make every serial port appear in the 4,64 ttyS namespace. > > If you remember correctly, that was my attempt at making serial port > minor assignment to be dynamic... just like everything else is today. > And it seemed to me that you thought this was a good idea. I may have thought that a dynamic space for serial devices was a good idea, but what I was referring to above was specifically the implementation. Unfortunately, there's precious little public evidence of this as the patches were never posted to a public mailing list. However, what there is (as part of another thread) shows that I held that view: http://archive.arm.linux.org.uk/lurker/message/20041124.164950.92dc25d7.en.html Plus, of course, the comments in the patch system where I picked out a number of further technical issues, such as changing inode->i_rdev, userspace locking, etc. If you want to review them, they're 1427/1 - 1434/1, 1435/2, 1436/2. Unfortunately the authorship of those comments was lost. Hence, my recollection is correct here.
On Mon, 27 Jan 2014, Russell King - ARM Linux wrote: > On Sun, Jan 26, 2014 at 11:30:00PM -0500, Nicolas Pitre wrote: > > On Sun, 26 Jan 2014, Russell King - ARM Linux wrote: > > > > > On Tue, Jan 21, 2014 at 12:45:05AM +0000, Alan Cox wrote: > > > > Peter handed it on. Try using git log on Documentation/devices.txt. It > > > > still gets updates. > > > > > > > > Perhaps you'd care to stick to reality and fix the tree instead of trying > > > > to excuse the mess ? > > > > > > Perhaps returning to reality might be advantageous rather than trying > > > to repeat statements which can't have any bearing on this - especially > > > as the git history which you're referring to only goes back to 2.6.12-rc2, > > > and this predates 2.6.12-rc2 by a long shot. > > > > > > > More importantly certain folks need to stop abusing static numbers > > > > allocated properly. Repeating it having made a total hash of it before > > > > is dismal. > > > > > > And if you continue these stupid accusations which have no basis at all, > > > we're going to get into a real big argument, because you are soo _wrong_ > > > on that point. I was always the one arguing /against/ the re-use of > > > existing major/minor numbers. I was the one arguing /against/ Nicolas' > > > patches to make every serial port appear in the 4,64 ttyS namespace. > > > > If you remember correctly, that was my attempt at making serial port > > minor assignment to be dynamic... just like everything else is today. > > And it seemed to me that you thought this was a good idea. > > I may have thought that a dynamic space for serial devices was a good > idea, but what I was referring to above was specifically the > implementation. Oh I do not dispute the fact that the implementation at the time was not up to needed standards. That would have been fixed eventually though, if it hadn't been for the policy based objection we received. At which point this effort was simply dropped. Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c index c1af04d..f8f41dc 100644 --- a/drivers/tty/serial/samsung.c +++ b/drivers/tty/serial/samsung.c @@ -1283,6 +1283,14 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) if (ret < 0) goto probe_err; + if (!s3c24xx_uart_drv.state) { + ret = uart_register_driver(&s3c24xx_uart_drv); + if (ret < 0) { + pr_err("Failed to register Samsung UART driver\n"); + return ret; + } + } + dbg("%s: adding port\n", __func__); uart_add_one_port(&s3c24xx_uart_drv, &ourport->port); platform_set_drvdata(pdev, &ourport->port); @@ -1315,6 +1323,8 @@ static int s3c24xx_serial_remove(struct platform_device *dev) uart_remove_one_port(&s3c24xx_uart_drv, port); } + uart_unregister_driver(&s3c24xx_uart_drv); + return 0; } @@ -1814,35 +1824,7 @@ static struct platform_driver samsung_serial_driver = { }, }; -/* module initialisation code */ - -static int __init s3c24xx_serial_modinit(void) -{ - int ret; - - ret = uart_register_driver(&s3c24xx_uart_drv); - if (ret < 0) { - pr_err("Failed to register Samsung UART driver\n"); - return ret; - } - - ret = platform_driver_register(&samsung_serial_driver); - if (ret < 0) { - pr_err("Failed to register platform driver\n"); - uart_unregister_driver(&s3c24xx_uart_drv); - } - - return ret; -} - -static void __exit s3c24xx_serial_modexit(void) -{ - platform_driver_unregister(&samsung_serial_driver); - uart_unregister_driver(&s3c24xx_uart_drv); -} - -module_init(s3c24xx_serial_modinit); -module_exit(s3c24xx_serial_modexit); +module_platform_driver(samsung_serial_driver); MODULE_ALIAS("platform:samsung-uart"); MODULE_DESCRIPTION("Samsung SoC Serial port driver");
uart_register_driver call binds the driver to a specific device node through tty_register_driver call. This should typically happen during device probe call. In a multiplatform scenario, it is possible that multiple serial drivers are part of the kernel. Currently the driver registration fails if multiple serial drivers with same default major/minor numbers are included in the kernel. A typical case is observed with amba-pl011 and samsung-uart drivers. Signed-off-by: Tushar Behera <tushar.behera@linaro.org> --- drivers/tty/serial/samsung.c | 40 +++++++++++----------------------------- 1 file changed, 11 insertions(+), 29 deletions(-)