Re: [U-Boot] [PATCH v4 2/2] serial: bcm283x_mu: Detect disabled serial device

[Alexander Graf]

> Am 16.08.2016 um 05:30 schrieb Stephen Warren :
> 
>> On 08/15/2016 09:48 AM, Alexander Graf wrote:
>> On the raspberry pi, you can disable the serial port to gain dynamic 
>> frequency
>> scaling which can get handy at times.
>> 
>> However, in such a configuration the serial controller gets its rx queue 
>> filled
>> up with zero bytes which then happily get transmitted on to whoever calls
>> getc() today.
>> 
>> This patch adds detection logic for that case by checking whether the RX pin 
>> is
>> mapped to GPIO15 and disables the mini uart if it is not mapped properly.
>> 
>> That way we can leave the driver enabled in the tree and can determine during
>> runtime whether serial is usable or not, having a single binary that allows 
>> for
>> uart and non-uart operation.
> 
> Acked-by: Stephen Warren 
> 
> Nits:
> 
> I'd hope for a core DM feature to disable statically created devices rather 
> than re-implementing it per driver, so we don't have to re-invent this each 
> time we need it. Still, we can refactor this later if it turns out to be more 
> generally useful.
> 
> Perhaps a separate patch for the raw serial driver feature 
> (serial_bcm283x_mu.h, serial_bcm283x_mu.c), and the board-specific logic (all 
> the other files)?

I was thinking about it, but figured that these two lines of code are so 
heavily intertwined with the fact that you want to actually set the disabled 
property. So I decided against splitting them out - it wouldn't benefit 
readability, bisectability or revertability (which are the usual reasons for 
splitting patches).


Alex


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v4 2/2] serial: bcm283x_mu: Detect disabled serial device

[Simon Glass]

On 15 August 2016 at 21:30, Stephen Warren  wrote:
>
> On 08/15/2016 09:48 AM, Alexander Graf wrote:
>>
>> On the raspberry pi, you can disable the serial port to gain dynamic 
>> frequency
>> scaling which can get handy at times.
>>
>> However, in such a configuration the serial controller gets its rx queue 
>> filled
>> up with zero bytes which then happily get transmitted on to whoever calls
>> getc() today.
>>
>> This patch adds detection logic for that case by checking whether the RX pin 
>> is
>> mapped to GPIO15 and disables the mini uart if it is not mapped properly.
>>
>> That way we can leave the driver enabled in the tree and can determine during
>> runtime whether serial is usable or not, having a single binary that allows 
>> for
>> uart and non-uart operation.
>
>
> Acked-by: Stephen Warren 
>
> Nits:
>
> I'd hope for a core DM feature to disable statically created devices rather 
> than re-implementing it per driver, so we don't have to re-invent this each 
> time we need it. Still, we can refactor this later if it turns out to be more 
> generally useful.
>

Agreed - yes let's wait until we have another case or two.

Reviewed-by: Simon Glass 

> Perhaps a separate patch for the raw serial driver feature 
> (serial_bcm283x_mu.h, serial_bcm283x_mu.c), and the board-specific logic (all 
> the other files)?

- Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v4 2/2] serial: bcm283x_mu: Detect disabled serial device

[Stephen Warren]

On 08/15/2016 09:48 AM, Alexander Graf wrote:

On the raspberry pi, you can disable the serial port to gain dynamic frequency
scaling which can get handy at times.

However, in such a configuration the serial controller gets its rx queue filled
up with zero bytes which then happily get transmitted on to whoever calls
getc() today.

This patch adds detection logic for that case by checking whether the RX pin is
mapped to GPIO15 and disables the mini uart if it is not mapped properly.

That way we can leave the driver enabled in the tree and can determine during
runtime whether serial is usable or not, having a single binary that allows for
uart and non-uart operation.


Acked-by: Stephen Warren 

Nits:

I'd hope for a core DM feature to disable statically created devices 
rather than re-implementing it per driver, so we don't have to re-invent 
this each time we need it. Still, we can refactor this later if it turns 
out to be more generally useful.


Perhaps a separate patch for the raw serial driver feature 
(serial_bcm283x_mu.h, serial_bcm283x_mu.c), and the board-specific logic 
(all the other files)?

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v4 2/2] serial: bcm283x_mu: Detect disabled serial device

[Alexander Graf]

On the raspberry pi, you can disable the serial port to gain dynamic frequency
scaling which can get handy at times.

However, in such a configuration the serial controller gets its rx queue filled
up with zero bytes which then happily get transmitted on to whoever calls
getc() today.

This patch adds detection logic for that case by checking whether the RX pin is
mapped to GPIO15 and disables the mini uart if it is not mapped properly.

That way we can leave the driver enabled in the tree and can determine during
runtime whether serial is usable or not, having a single binary that allows for
uart and non-uart operation.

Signed-off-by: Alexander Graf 

---

v2 -> v3:

  - Disable and detect pinmux in board file

v3 -> v4:

  - Detect in early code and disable by failing serial probe
---
 board/raspberrypi/rpi/rpi.c  | 34 +++-
 configs/rpi_3_32b_defconfig  |  2 ++
 configs/rpi_3_defconfig  |  2 ++
 drivers/gpio/bcm2835_gpio.c  |  1 +
 drivers/serial/serial_bcm283x_mu.c   |  3 +++
 include/configs/rpi.h|  1 +
 include/dm/platform_data/serial_bcm283x_mu.h |  1 +
 7 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index 4c8253d..7f057e1 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -51,7 +51,7 @@ U_BOOT_DEVICE(bcm2835_serials) = {
.platdata = _platdata,
 };
 #else
-static const struct bcm283x_mu_serial_platdata serial_platdata = {
+static struct bcm283x_mu_serial_platdata serial_platdata = {
.base = 0x3f215040,
.clock = 25000,
.skip_init = true,
@@ -453,6 +453,38 @@ int board_init(void)
return power_on_module(BCM2835_MBOX_POWER_DEVID_USB_HCD);
 }
 
+#ifndef CONFIG_PL01X_SERIAL
+static bool rpi_is_serial_active(void)
+{
+   int serial_gpio = 15;
+   struct udevice *dev;
+
+   /*
+* The RPi3 disables the mini uart by default. The easiest way to find
+* out whether it is available is to check if the RX pin is muxed.
+*/
+
+   if (uclass_first_device(UCLASS_GPIO, ) || !dev)
+   return true;
+
+   if (bcm2835_gpio_get_func_id(dev, serial_gpio) != BCM2835_GPIO_ALT5)
+   return false;
+
+   return true;
+}
+#endif
+
+int board_early_init_f(void)
+{
+#ifndef CONFIG_PL01X_SERIAL
+   /* Disable mini-UART I/O if it's not pinmuxed to our pins */
+   if (!rpi_is_serial_active())
+   serial_platdata.disabled = true;
+#endif
+
+   return 0;
+}
+
 int board_mmc_init(bd_t *bis)
 {
ALLOC_CACHE_ALIGN_BUFFER(struct msg_get_clock_rate, msg_clk, 1);
diff --git a/configs/rpi_3_32b_defconfig b/configs/rpi_3_32b_defconfig
index 922e01b..c59474c 100644
--- a/configs/rpi_3_32b_defconfig
+++ b/configs/rpi_3_32b_defconfig
@@ -20,3 +20,5 @@ CONFIG_CMD_FAT=y
 CONFIG_CMD_FS_GENERIC=y
 CONFIG_PHYS_TO_BUS=y
 CONFIG_OF_LIBFDT=y
+# CONFIG_REQUIRE_SERIAL_CONSOLE is not set
+CONFIG_SYS_MALLOC_F_LEN=0x2000
diff --git a/configs/rpi_3_defconfig b/configs/rpi_3_defconfig
index bff92df..67c4a0c 100644
--- a/configs/rpi_3_defconfig
+++ b/configs/rpi_3_defconfig
@@ -19,3 +19,5 @@ CONFIG_CMD_FAT=y
 CONFIG_CMD_FS_GENERIC=y
 CONFIG_PHYS_TO_BUS=y
 CONFIG_OF_LIBFDT=y
+# CONFIG_REQUIRE_SERIAL_CONSOLE is not set
+CONFIG_SYS_MALLOC_F_LEN=0x2000
diff --git a/drivers/gpio/bcm2835_gpio.c b/drivers/gpio/bcm2835_gpio.c
index 8b88d79..8dd7a28 100644
--- a/drivers/gpio/bcm2835_gpio.c
+++ b/drivers/gpio/bcm2835_gpio.c
@@ -123,5 +123,6 @@ U_BOOT_DRIVER(gpio_bcm2835) = {
.id = UCLASS_GPIO,
.ops= _bcm2835_ops,
.probe  = bcm2835_gpio_probe,
+   .flags  = DM_FLAG_PRE_RELOC,
.priv_auto_alloc_size = sizeof(struct bcm2835_gpios),
 };
diff --git a/drivers/serial/serial_bcm283x_mu.c 
b/drivers/serial/serial_bcm283x_mu.c
index 7357bbf..f4e062f 100644
--- a/drivers/serial/serial_bcm283x_mu.c
+++ b/drivers/serial/serial_bcm283x_mu.c
@@ -73,6 +73,9 @@ static int bcm283x_mu_serial_probe(struct udevice *dev)
struct bcm283x_mu_serial_platdata *plat = dev_get_platdata(dev);
struct bcm283x_mu_priv *priv = dev_get_priv(dev);
 
+   if (plat->disabled)
+   return -ENODEV;
+
priv->regs = (struct bcm283x_mu_regs *)plat->base;
 
return 0;
diff --git a/include/configs/rpi.h b/include/configs/rpi.h
index b5543f4..4e5b3f3 100644
--- a/include/configs/rpi.h
+++ b/include/configs/rpi.h
@@ -22,6 +22,7 @@
 
 /* Architecture, CPU, etc.*/
 #define CONFIG_ARCH_CPU_INIT
+#define CONFIG_BOARD_EARLY_INIT_F
 
 /* Use SoC timer for AArch32, but architected timer for AArch64 */
 #ifndef CONFIG_ARM64
diff --git a/include/dm/platform_data/serial_bcm283x_mu.h 
b/include/dm/platform_data/serial_bcm283x_mu.h
index 57ae6ad..c47d3c0 100644
--- a/include/dm/platform_data/serial_bcm283x_mu.h
+++ b/include/dm/platform_data/serial_bcm283x_mu.h
@@