[v2,2/7] serial: Allow boards to determine whether a serial device is usable

Message ID 20180117085458.27293-3-agraf@suse.de
State New
Headers show
Series
  • RPi: Properly handle dynamic serial configuration
Related show

Commit Message

Alexander Graf Jan. 17, 2018, 8:54 a.m.
On some boards, serial devices may or may not be muxed properly to actual
pins, depending on firmware configuration. To determine whether we should
use a serial device for U-Boot in-/output, we need to check whether it
is muxed properly.

This is something only the board file can do, so let's expose a weak
function that a board can override to explicitly allow or disallow
usage of certain serial devices.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 drivers/serial/serial-uclass.c | 11 +++++++++++
 include/serial.h               | 11 +++++++++++
 2 files changed, 22 insertions(+)

Comments

Simon Glass Jan. 17, 2018, 7:39 p.m. | #1
Hi Alex,

On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
> On some boards, serial devices may or may not be muxed properly to actual
> pins, depending on firmware configuration. To determine whether we should
> use a serial device for U-Boot in-/output, we need to check whether it
> is muxed properly.
>
> This is something only the board file can do, so let's expose a weak
> function that a board can override to explicitly allow or disallow
> usage of certain serial devices.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  drivers/serial/serial-uclass.c | 11 +++++++++++
>  include/serial.h               | 11 +++++++++++
>  2 files changed, 22 insertions(+)
>

Can we please figure out how handle this in the serial driver / driver
model itself? I want to avoid weak functions with driver model.

Regards,
Simon
Alexander Graf Jan. 17, 2018, 10:03 p.m. | #2
On 17.01.18 20:39, Simon Glass wrote:
> Hi Alex,
> 
> On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
>> On some boards, serial devices may or may not be muxed properly to actual
>> pins, depending on firmware configuration. To determine whether we should
>> use a serial device for U-Boot in-/output, we need to check whether it
>> is muxed properly.
>>
>> This is something only the board file can do, so let's expose a weak
>> function that a board can override to explicitly allow or disallow
>> usage of certain serial devices.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>  drivers/serial/serial-uclass.c | 11 +++++++++++
>>  include/serial.h               | 11 +++++++++++
>>  2 files changed, 22 insertions(+)
>>
> 
> Can we please figure out how handle this in the serial driver / driver
> model itself? I want to avoid weak functions with driver model.

I'm very happy to see suggestions :). The reason I went with the weak
function is really because I couldn't think of anything better.


Alex
Simon Glass Jan. 17, 2018, 10:11 p.m. | #3
Hi Alex,

On 17 January 2018 at 15:03, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 17.01.18 20:39, Simon Glass wrote:
>> Hi Alex,
>>
>> On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
>>> On some boards, serial devices may or may not be muxed properly to actual
>>> pins, depending on firmware configuration. To determine whether we should
>>> use a serial device for U-Boot in-/output, we need to check whether it
>>> is muxed properly.
>>>
>>> This is something only the board file can do, so let's expose a weak
>>> function that a board can override to explicitly allow or disallow
>>> usage of certain serial devices.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>>  drivers/serial/serial-uclass.c | 11 +++++++++++
>>>  include/serial.h               | 11 +++++++++++
>>>  2 files changed, 22 insertions(+)
>>>
>>
>> Can we please figure out how handle this in the serial driver / driver
>> model itself? I want to avoid weak functions with driver model.
>
> I'm very happy to see suggestions :). The reason I went with the weak
> function is really because I couldn't think of anything better.

The serial driver is proprietary so you should be able to put a call
into the pinctrl driver from that. The pinctrl driver can return the
state of a pin - maybe get_gpio_mux() or a new pinmux_get()?

Regards,
Simon
Alexander Graf Jan. 17, 2018, 10:37 p.m. | #4
On 17.01.18 23:11, Simon Glass wrote:
> Hi Alex,
> 
> On 17 January 2018 at 15:03, Alexander Graf <agraf@suse.de> wrote:
>>
>>
>> On 17.01.18 20:39, Simon Glass wrote:
>>> Hi Alex,
>>>
>>> On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
>>>> On some boards, serial devices may or may not be muxed properly to actual
>>>> pins, depending on firmware configuration. To determine whether we should
>>>> use a serial device for U-Boot in-/output, we need to check whether it
>>>> is muxed properly.
>>>>
>>>> This is something only the board file can do, so let's expose a weak
>>>> function that a board can override to explicitly allow or disallow
>>>> usage of certain serial devices.
>>>>
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> ---
>>>>  drivers/serial/serial-uclass.c | 11 +++++++++++
>>>>  include/serial.h               | 11 +++++++++++
>>>>  2 files changed, 22 insertions(+)
>>>>
>>>
>>> Can we please figure out how handle this in the serial driver / driver
>>> model itself? I want to avoid weak functions with driver model.
>>
>> I'm very happy to see suggestions :). The reason I went with the weak
>> function is really because I couldn't think of anything better.
> 
> The serial driver is proprietary so you should be able to put a call
> into the pinctrl driver from that. The pinctrl driver can return the
> state of a pin - maybe get_gpio_mux() or a new pinmux_get()?

The SoC has 2 serial drivers: a proprietary one and a pl011. Would you
think it's ok to put an architecture specific hack into the generic
pl011 code with an #ifdef?


Alex
Simon Glass Jan. 17, 2018, 11:18 p.m. | #5
Hi Alex,

On 17 January 2018 at 14:37, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 17.01.18 23:11, Simon Glass wrote:
>> Hi Alex,
>>
>> On 17 January 2018 at 15:03, Alexander Graf <agraf@suse.de> wrote:
>>>
>>>
>>> On 17.01.18 20:39, Simon Glass wrote:
>>>> Hi Alex,
>>>>
>>>> On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
>>>>> On some boards, serial devices may or may not be muxed properly to actual
>>>>> pins, depending on firmware configuration. To determine whether we should
>>>>> use a serial device for U-Boot in-/output, we need to check whether it
>>>>> is muxed properly.
>>>>>
>>>>> This is something only the board file can do, so let's expose a weak
>>>>> function that a board can override to explicitly allow or disallow
>>>>> usage of certain serial devices.
>>>>>
>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>> ---
>>>>>  drivers/serial/serial-uclass.c | 11 +++++++++++
>>>>>  include/serial.h               | 11 +++++++++++
>>>>>  2 files changed, 22 insertions(+)
>>>>>
>>>>
>>>> Can we please figure out how handle this in the serial driver / driver
>>>> model itself? I want to avoid weak functions with driver model.
>>>
>>> I'm very happy to see suggestions :). The reason I went with the weak
>>> function is really because I couldn't think of anything better.
>>
>> The serial driver is proprietary so you should be able to put a call
>> into the pinctrl driver from that. The pinctrl driver can return the
>> state of a pin - maybe get_gpio_mux() or a new pinmux_get()?
>
> The SoC has 2 serial drivers: a proprietary one and a pl011. Would you
> think it's ok to put an architecture specific hack into the generic
> pl011 code with an #ifdef?

Normally in this situation we create an outer driver which uses the
other one - see serial_rockchip.c for example. That way the base
driver doesn't have to know about arch-specific things.

Regards,
Simon

Patch

diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 68ca2d09d1..ecd64f8e86 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -366,6 +366,16 @@  static int on_baudrate(const char *name, const char *value, enum env_op op,
 U_BOOT_ENV_CALLBACK(baudrate, on_baudrate);
 
 #if CONFIG_IS_ENABLED(SERIAL_PRESENT)
+__weak int board_check_serial(struct udevice *dev)
+{
+	return 0;
+}
+
+static int serial_pre_probe(struct udevice *dev)
+{
+	return board_check_serial(dev);
+}
+
 static int serial_post_probe(struct udevice *dev)
 {
 	struct dm_serial_ops *ops = serial_get_ops(dev);
@@ -438,6 +448,7 @@  UCLASS_DRIVER(serial) = {
 	.name		= "serial",
 	.flags		= DM_UC_FLAG_SEQ_ALIAS,
 	.post_probe	= serial_post_probe,
+	.pre_probe	= serial_pre_probe,
 	.pre_remove	= serial_pre_remove,
 	.per_device_auto_alloc_size = sizeof(struct serial_dev_priv),
 };
diff --git a/include/serial.h b/include/serial.h
index d87f01082a..221b3e1402 100644
--- a/include/serial.h
+++ b/include/serial.h
@@ -207,4 +207,15 @@  void sh_serial_initialize(void);
 void uartlite_serial_initialize(void);
 void zynq_serial_initialize(void);
 
+/**
+ * board_check_serial() - Determine whether a serial device works
+ *
+ * This is a board callback that allows boards to override whether a serial
+ * device is usable. By default, all devices are declared usable.
+ *
+ * @dev: Device pointer
+ * @return 0 if the device is usable, !0 otherwise
+ */
+int board_check_serial(struct udevice *dev);
+
 #endif