tty/serial: at91: restore dynamic driver binding
Subsystems (3)
PATCH v2 1/1 tty/serial: at91: restore dynamic driver binding
In commit c39dfebc7798956fd2140ae6321786ff35da30c3, the modular support code for atmel_serial was removed, as the driver cannot be built as a module. Because no use case was proposed, the dynamic driver binding support was removed as well.
The atmel_serial driver can manage up to 7 serial controllers, which are multiplexed with other functions. For example, in the Atmel SAMA5D2, the Flexcom controllers can work as USART, SPI or I2C controllers, and on all Atmel devices serial lines can be reconfigured as GPIOs.
My use case uses GPIOs to transfer a firmware update using a custom protocol on the lines used as a serial port during the normal life of the device. If it is not possible to unbind the atmel_serial driver, the GPIO lines remain reserved and prevent this case from working.
This patch reinstates the atmel_serial_remove function, and fixes it as it failed to clear the "clk" field on removal, triggering an oops when a device was bound again after being unbound.
---
Changelog:
v2: Add the rationale for keeping the "remove" function as a comment.
drivers/tty/serial/atmel_serial.c | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-)
drivers/tty/serial/atmel_serial.c +36 -3
@@ -2759,14 +2759,47 @@ err:return ret; } +/* +* Even if the driver is not modular, it makes sense to be able to +* unbind a device: there can be many bound devices, and there are +* situations where dynamic binding and unbinding can be useful. +* +* For example, a connected device can require a specific firmware update +* protocol that needs bitbanging on IO lines, but use the regular serial +* port in the normal case. +*/ +static int atmel_serial_remove (struct platform_device *pdev) +{ +struct uart_port *port =platform_get_drvdata (pdev); +struct atmel_uart_port *atmel_port =to_atmel_uart_port (port); +int ret =0 ; + +tasklet_kill (&atmel_port->tasklet); + +device_init_wakeup (&pdev->dev,0 ); + + ret =uart_remove_one_port (&atmel_uart, port); + +kfree (atmel_port->rx_ring.buf); + +/* "port" is allocated statically, so we shouldn't free it */ + +clear_bit (port->line, atmel_ports_in_use); + +clk_put (atmel_port->clk); + atmel_port->clk =NULL ; + +return ret; +} +static struct platform_driver atmel_serial_driver = { .probe = atmel_serial_probe, + .remove = atmel_serial_remove, .suspend = atmel_serial_suspend, .resume = atmel_serial_resume, .driver = { - .name ="atmel_usart" , - .of_match_table =of_match_ptr (atmel_serial_dt_ids), - .suppress_bind_attrs = true, + .name ="atmel_usart" , + .of_match_table =of_match_ptr (atmel_serial_dt_ids), }, };
Signature · 1 line
2.5.0
Quoted 15 lines · Romain Izard
In commit
c39dfebc7798956fd2140ae6321786ff35da30c3, the modular support code foratmel_serialwas removed, as the driver cannot be built as a module. Because no use case was proposed, the dynamic driver binding support was removed as well.The
atmel_serialdriver can manage up to 7 serial controllers, which are multiplexed with other functions. For example, in the Atmel SAMA5D2, the Flexcom controllers can work as USART, SPI or I2C controllers, and on all Atmel devices serial lines can be reconfigured as GPIOs.My use case uses GPIOs to transfer a firmware update using a custom protocol on the lines used as a serial port during the normal life of the device. If it is not possible to unbind the
atmel_serialdriver, the GPIO lines remain reserved and prevent this case from working.
Quoted 5 lines
This patch reinstates the
atmel_serial_removefunction, and fixes it as it failed to clear the "clk" field on removal, triggering an oops when a device was bound again after being unbound.Signed-off-by Romain Izard <[email protected]>
Even if you didn't follow my advice for not including unneeded changes in of the last patch chunk, there's no use delaying the patch just for this. So, here is my:
Thanks, bye.
PS: another advice: please include linux-arm-kernel as well in your CC list for work on the AT91 platforms: most of the reviewers are there.
Quoted 64 lines
---
Changelog:
v2: Add the rationale for keeping the "remove" function as a comment.drivers/tty/serial/atmel_serial.c | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-)drivers/tty/serial/atmel_serial.c +36
-3@@ -2759,14 +2759,47 @@ err:return ret; } +/* +* Even if the driver is not modular, it makes sense to be able to +* unbind a device: there can be many bound devices, and there are +* situations where dynamic binding and unbinding can be useful. +* +* For example, a connected device can require a specific firmware update +* protocol that needs bitbanging on IO lines, but use the regular serial +* port in the normal case. +*/ +static int atmel_serial_remove (struct platform_device *pdev) +{ +struct uart_port *port =platform_get_drvdata (pdev); +struct atmel_uart_port *atmel_port =to_atmel_uart_port (port); +int ret =0 ; + +tasklet_kill (&atmel_port->tasklet); + +device_init_wakeup (&pdev->dev,0 ); + + ret =uart_remove_one_port (&atmel_uart, port); + +kfree (atmel_port->rx_ring.buf); + +/* "port" is allocated statically, so we shouldn't free it */ + +clear_bit (port->line, atmel_ports_in_use); + +clk_put (atmel_port->clk); + atmel_port->clk =NULL ; + +return ret; +} +static struct platform_driver atmel_serial_driver = { .probe = atmel_serial_probe, + .remove = atmel_serial_remove, .suspend = atmel_serial_suspend, .resume = atmel_serial_resume, .driver = { - .name ="atmel_usart" , - .of_match_table =of_match_ptr (atmel_serial_dt_ids), - .suppress_bind_attrs = true, + .name ="atmel_usart" , + .of_match_table =of_match_ptr (atmel_serial_dt_ids), }, };
Signature · 1 line
Nicolas Ferre
Greg Kroah-Hartman
maintainerQuoted 20 lines · Nicolas Ferre
Le 25/02/2016 10:23, Romain Izard a ?crit :
Quoted 19 lines
In commit
c39dfebc7798956fd2140ae6321786ff35da30c3, the modular support code foratmel_serialwas removed, as the driver cannot be built as a module. Because no use case was proposed, the dynamic driver binding support was removed as well.The
atmel_serialdriver can manage up to 7 serial controllers, which are multiplexed with other functions. For example, in the Atmel SAMA5D2, the Flexcom controllers can work as USART, SPI or I2C controllers, and on all Atmel devices serial lines can be reconfigured as GPIOs.My use case uses GPIOs to transfer a firmware update using a custom protocol on the lines used as a serial port during the normal life of the device. If it is not possible to unbind the
atmel_serialdriver, the GPIO lines remain reserved and prevent this case from working.This patch reinstates the
atmel_serial_removefunction, and fixes it as it failed to clear the "clk" field on removal, triggering an oops when a device was bound again after being unbound.
Quoted 5 lines
Quoted 1 line
Signed-off-by Romain Izard <[email protected]>Even if you didn't follow my advice for not including unneeded changes in of the last patch chunk, there's no use delaying the patch just for this. So, here is my:
Yes there is, I'm not going to take this, Romain please fix it properly.
greg k-h
2016-02-25 17:09 GMT+01:00 Greg Kroah-Hartman
<[email protected]>:
Quoted 23 lines · Nicolas Ferre
Quoted 22 lines
Le 25/02/2016 10:23, Romain Izard a ?crit :
Quoted 21 lines
In commit
c39dfebc7798956fd2140ae6321786ff35da30c3, the modular support code foratmel_serialwas removed, as the driver cannot be built as a module. Because no use case was proposed, the dynamic driver binding support was removed as well.The
atmel_serialdriver can manage up to 7 serial controllers, which are multiplexed with other functions. For example, in the Atmel SAMA5D2, the Flexcom controllers can work as USART, SPI or I2C controllers, and on all Atmel devices serial lines can be reconfigured as GPIOs.My use case uses GPIOs to transfer a firmware update using a custom protocol on the lines used as a serial port during the normal life of the device. If it is not possible to unbind the
atmel_serialdriver, the GPIO lines remain reserved and prevent this case from working.This patch reinstates the
atmel_serial_removefunction, and fixes it as it failed to clear the "clk" field on removal, triggering an oops when a device was bound again after being unbound.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Signed-off-by Romain Izard <[email protected]>Even if you didn't follow my advice for not including unneeded changes in of the last patch chunk, there's no use delaying the patch just for this. So, here is my:
Yes there is, I'm not going to take this, Romain please fix it properly.
Are we really arguing about the alignement of of_match_table in the platform_driver initializer?
Among other things, Paul's patch changed the alignment to match the width of the "suppress_bind_attrs" member. As I simply used 'git revert -p' to revert the parts of the patch that bothered me, the alignment returned to what it was before.
Or am I missing something else ?
Signature · 1 line
Romain Izard
Quoted 37 lines · Romain Izard
2016-02-25 17:09 GMT+01:00 Greg Kroah-Hartman
<[email protected]>:Quoted 23 lines · Nicolas Ferre
Quoted 22 lines · Romain Izard
Quoted 21 lines
In commit
c39dfebc7798956fd2140ae6321786ff35da30c3, the modular support code foratmel_serialwas removed, as the driver cannot be built as a module. Because no use case was proposed, the dynamic driver binding support was removed as well.The
atmel_serialdriver can manage up to 7 serial controllers, which are multiplexed with other functions. For example, in the Atmel SAMA5D2, the Flexcom controllers can work as USART, SPI or I2C controllers, and on all Atmel devices serial lines can be reconfigured as GPIOs.My use case uses GPIOs to transfer a firmware update using a custom protocol on the lines used as a serial port during the normal life of the device. If it is not possible to unbind the
atmel_serialdriver, the GPIO lines remain reserved and prevent this case from working.This patch reinstates the
atmel_serial_removefunction, and fixes it as it failed to clear the "clk" field on removal, triggering an oops when a device was bound again after being unbound.Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Signed-off-by Romain Izard <[email protected]>Even if you didn't follow my advice for not including unneeded changes in of the last patch chunk, there's no use delaying the patch just for this. So, here is my:
Yes there is, I'm not going to take this, Romain please fix it properly.
Are we really arguing about the alignement of
of_match_tablein theplatform_driverinitializer?
Quoted 6 lines
Among other things, Paul's patch changed the alignment to match the width of the "
suppress_bind_attrs" member. As I simply used 'git revert -p' to revert the parts of the patch that bothered me, the alignment returned to what it was before.Or am I missing something else ?
Romain,
We are just saying that a patch with:
- .name = "atmel_usart",
- .of_match_table = of_match_ptr(atmel_serial_dt_ids),
- .suppress_bind_attrs = true,
+ .name = "atmel_usart",
+ .of_match_table = of_match_ptr(atmel_serial_dt_ids),
Is less readable than a patch with only the relevant part, the single line:
- .suppress_bind_attrs = true,
So, whichever is the history of the patch, it has to simply modify the needed lines so that we don't even ask ourselves what is the purpose of some of the changes.
Bye,
Signature · 1 line
Nicolas Ferre