soc/rockchip: add handler for usb-uart functionality

Most newer Rockchip SoCs provide the possibility to use a usb-phy
as passthrough for the debug uart (uart2), making it possible to
get console output without needing to open the device.

This patch adds an early_initcall to enable this functionality
conditionally via the commandline and also disables the corresponding
usb controller in the devicetree.

Currently only data for the rk3288 is provided, but at least the
rk3188 and arm64 rk3368 also provide this functionality and will be
enabled later.

On a spliced usb cable the signals are tx on white wire(D+) and
rx on green wire(D-).

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
changes from the version in chromium-review:
- enable siddq (probably saves a little power to power down plls)
- cosmetics to make checkpatch happy

Yunzhi Li already gave a Code-Review+1 in the chromium gerrit
but I'd guess a public Reviewed-by would be better ;-)


 Documentation/kernel-parameters.txt      |   6 +
 drivers/soc/Kconfig                      |   1 +
 drivers/soc/Makefile                     |   1 +
 drivers/soc/rockchip/Kconfig             |  13 ++
 drivers/soc/rockchip/Makefile            |   1 +
 drivers/soc/rockchip/rockchip_usb_uart.c | 226 +++++++++++++++++++++++++++++++
 6 files changed, 248 insertions(+)
 create mode 100644 drivers/soc/rockchip/Kconfig
 create mode 100644 drivers/soc/rockchip/Makefile
 create mode 100644 drivers/soc/rockchip/rockchip_usb_uart.c

Am Mittwoch, 8. Juli 2015, 21:00:06 schrieb Heiko Stübner:
> Most newer Rockchip SoCs provide the possibility to use a usb-phy
> as passthrough for the debug uart (uart2), making it possible to
> get console output without needing to open the device.
> 
> This patch adds an early_initcall to enable this functionality
> conditionally via the commandline and also disables the corresponding
> usb controller in the devicetree.
> 
> Currently only data for the rk3288 is provided, but at least the
> rk3188 and arm64 rk3368 also provide this functionality and will be
> enabled later.
> 
> On a spliced usb cable the signals are tx on white wire(D+) and
> rx on green wire(D-).
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

dumping some thoughts that stem from comments in my g+ post about this:


Arnd Bergmann, yesterday 19:40:
Would it help to move this code into the console init section of the UART 
driver to avoid losing the initial output?


Heiko Stübner, yesterday 20:08
I don't think so. The functionality has nothing to do with the uart itself - 
in fact for the system everything stays the same - only the routing becomes 
different. It should even work with earlyprintk/earlycon.

The only output you loose is up to the point when the early_initcall is 
executed - which is around the smp bringup time. And I guess if somebody is 
doing kernel changes for this early stage, they will probably use a board with 
a real uart or modify their uboot to already also set this option.


Arnd Bergmann, yesterday 20:52
Isn't early_initcall /much/ later than console_init, or even earlycon?
The console output gets buffered until the first console gets found and is then 
flushed to that. If you switch the MUX just before registering the console, you 
should not lose anything, and also put this code in a more natural location.


Heiko Stübner, today 23:03
Yep as I said, twi switch currently runs around the time of the smp bringup . 
I'm probably just daft, but also still not sure how this should fit into the 
uart driver/console stuff. Especially as it is not even a property of the 
(designware) uart ips, but instead it might even be more a property of the usb 
phy. In terms of natural location, the usbphy driver might then be a more 
suitable position?

In terms of when to run, it also looks like early_initcall is one of the 
earliest times to run, as the code needs syscons, devicetree matching and also 
devicetree updates.

If just the regular console is included, nothing is lost and I guess when 
somebody needs to touch (and debug) such low-low-level parts like before the 
smp bringup they won't do it on a board without a real serial port I guess.

I guess we should also move this to a mailing list ... I guess I'll just copy 
our thread here into a reply to my patch ;-) .

2015-07-08 21:00 GMT+02:00 Heiko Stübner <heiko@sntech.de>:
> Most newer Rockchip SoCs provide the possibility to use a usb-phy
> as passthrough for the debug uart (uart2), making it possible to
> get console output without needing to open the device.
>
> This patch adds an early_initcall to enable this functionality
> conditionally via the commandline and also disables the corresponding
> usb controller in the devicetree.
>
> Currently only data for the rk3288 is provided, but at least the
> rk3188 and arm64 rk3368 also provide this functionality and will be
> enabled later.
>
> On a spliced usb cable the signals are tx on white wire(D+) and
> rx on green wire(D-).
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> changes from the version in chromium-review:
> - enable siddq (probably saves a little power to power down plls)
> - cosmetics to make checkpatch happy
>
> Yunzhi Li already gave a Code-Review+1 in the chromium gerrit
> but I'd guess a public Reviewed-by would be better ;-)
>
>
>  Documentation/kernel-parameters.txt      |   6 +
>  drivers/soc/Kconfig                      |   1 +
>  drivers/soc/Makefile                     |   1 +
>  drivers/soc/rockchip/Kconfig             |  13 ++
>  drivers/soc/rockchip/Makefile            |   1 +
>  drivers/soc/rockchip/rockchip_usb_uart.c | 226 +++++++++++++++++++++++++++++++
>  6 files changed, 248 insertions(+)
>  create mode 100644 drivers/soc/rockchip/Kconfig
>  create mode 100644 drivers/soc/rockchip/Makefile
>  create mode 100644 drivers/soc/rockchip/rockchip_usb_uart.c
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 1d6f045..156911b 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -3339,6 +3339,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>
>         ro              [KNL] Mount root device read-only on boot
>
> +       rockchip.usb_uart
> +                       Enable the uart passthrough on the designated usb port
> +                       on Rockchip SoCs. When active, the signals of the
> +                       debug-uart get routed to the D+ and D- pins of the usb
> +                       port and the regular usb controller gets disabled.
> +
>         root=           [KNL] Root filesystem
>                         See name_to_dev_t comment in init/do_mounts.c.
>
> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> index 96ddecb..ecb1a6c 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -2,6 +2,7 @@ menu "SOC (System On Chip) specific Drivers"
>
>  source "drivers/soc/mediatek/Kconfig"
>  source "drivers/soc/qcom/Kconfig"
> +source "drivers/soc/rockchip/Kconfig"
>  source "drivers/soc/sunxi/Kconfig"
>  source "drivers/soc/ti/Kconfig"
>  source "drivers/soc/versatile/Kconfig"
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 7dc7c0d..6c0b677 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -4,6 +4,7 @@
>
>  obj-$(CONFIG_ARCH_MEDIATEK)    += mediatek/
>  obj-$(CONFIG_ARCH_QCOM)                += qcom/
> +obj-$(CONFIG_ARCH_ROCKCHIP)    += rockchip/
>  obj-$(CONFIG_ARCH_SUNXI)       += sunxi/
>  obj-$(CONFIG_ARCH_TEGRA)       += tegra/
>  obj-$(CONFIG_SOC_TI)           += ti/
> diff --git a/drivers/soc/rockchip/Kconfig b/drivers/soc/rockchip/Kconfig
> new file mode 100644
> index 0000000..24d4e05
> --- /dev/null
> +++ b/drivers/soc/rockchip/Kconfig
> @@ -0,0 +1,13 @@
> +#
> +# Rockchip Soc drivers
> +#
> +config ROCKCHIP_USB_UART
> +       bool "Rockchip usb-uart override"
> +       depends on ARCH_ROCKCHIP
> +       select MFD_SYSCON
> +       help
> +         Say y here to enable usb-uart functionality. Newer Rockchip SoCs
> +         provide means to repurpose one usb phy as uart2 output, making it
> +         possible to get debug output without needing to open a device.
> +         To enable this function on boot, add a rockchip.usb_uart option
> +         to the kernel commandline.
> diff --git a/drivers/soc/rockchip/Makefile b/drivers/soc/rockchip/Makefile
> new file mode 100644
> index 0000000..b5dd6f8
> --- /dev/null
> +++ b/drivers/soc/rockchip/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_ROCKCHIP_USB_UART)                +=      rockchip_usb_uart.o
> diff --git a/drivers/soc/rockchip/rockchip_usb_uart.c b/drivers/soc/rockchip/rockchip_usb_uart.c
> new file mode 100644
> index 0000000..8e96d46
> --- /dev/null
> +++ b/drivers/soc/rockchip/rockchip_usb_uart.c
> @@ -0,0 +1,226 @@
> +/*
> + * Copyright (c) 2015 Heiko Stuebner <heiko@sntech.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#define HIWORD_UPDATE(val, mask) \
> +               ((val) | (mask) << 16)
> +
> +struct rockchip_uart_data {
> +       const char *grf_compatible;
> +       const char *usb_compatible;
> +       phys_addr_t usb_phys_addr;
> +       int (*init_uart)(const struct rockchip_uart_data *data,
> +                        struct regmap *grf);
> +};
> +
> +static int enable_usb_uart;
> +
> +#define RK3288_UOC0_CON0                               0x320
> +#define RK3288_UOC0_CON0_COMMON_ON_N                   BIT(0)
> +#define RK3288_UOC0_CON0_DISABLE                       BIT(4)
> +#define RK3288_UOC0_CON0_SIDDQ                         BIT(13)
> +
> +#define RK3288_UOC0_CON2                               0x328
> +#define RK3288_UOC0_CON2_SOFT_CON_SEL                  BIT(2)
> +
> +#define RK3288_UOC0_CON3                               0x32c
> +#define RK3288_UOC0_CON3_UTMI_SUSPENDN                 BIT(0)
> +#define RK3288_UOC0_CON3_UTMI_OPMODE_NODRIVING         (1 << 1)
> +#define RK3288_UOC0_CON3_UTMI_OPMODE_MASK              (3 << 1)
> +#define RK3288_UOC0_CON3_UTMI_XCVRSEELCT_FSTRANSC      (1 << 3)
> +#define RK3288_UOC0_CON3_UTMI_XCVRSEELCT_MASK          (3 << 3)
> +#define RK3288_UOC0_CON3_UTMI_TERMSEL_FULLSPEED                BIT(5)
> +#define RK3288_UOC0_CON3_BYPASSDMEN                    BIT(6)
> +#define RK3288_UOC0_CON3_BYPASSSEL                     BIT(7)

I was wondering why these values are "hardcoded" directly, but as you
said, support for other SoCs will be enabled later.
Keep things simple as beginning is okay for me.

> +
> +/*
> + * Enable the bypass of uart2 data through the otg usb phy.
> + * Original description in the TRM.
> + * 1. Disable the OTG block by setting OTGDISABLE0 to 1’b1.
> + * 2. Disable the pull-up resistance on the D+ line by setting
> + *    OPMODE0[1:0] to 2’b01.
> + * 3. To ensure that the XO, Bias, and PLL blocks are powered down in Suspend
> + *    mode, set COMMONONN to 1’b1.
> + * 4. Place the USB PHY in Suspend mode by setting SUSPENDM0 to 1’b0.
> + * 5. Set BYPASSSEL0 to 1’b1.
> + * 6. To transmit data, controls BYPASSDMEN0, and BYPASSDMDATA0.
> + * To receive data, monitor FSVPLUS0.
> + *
> + * The actual code in the vendor kernel does some things differently.
> + */
> +static int __init rk3288_init_usb_uart(const struct rockchip_uart_data *data,
> +                                      struct regmap *grf)
> +{
> +       u32 val;
> +       int ret;
> +
> +       /*
> +        * COMMON_ON and DISABLE settings are described in the TRM,
> +        * but were not present in the original code.
> +        * Also disable the analog phy components to save power.
> +        */
> +       val = HIWORD_UPDATE(RK3288_UOC0_CON0_COMMON_ON_N
> +                               | RK3288_UOC0_CON0_DISABLE
> +                               | RK3288_UOC0_CON0_SIDDQ,
> +                           RK3288_UOC0_CON0_COMMON_ON_N
> +                               | RK3288_UOC0_CON0_DISABLE
> +                               | RK3288_UOC0_CON0_SIDDQ);
> +       ret = regmap_write(grf, RK3288_UOC0_CON0, val);
> +       if (ret)
> +               return ret;
> +
> +       val = HIWORD_UPDATE(RK3288_UOC0_CON2_SOFT_CON_SEL,
> +                           RK3288_UOC0_CON2_SOFT_CON_SEL);
> +       ret = regmap_write(grf, RK3288_UOC0_CON2, val);
> +       if (ret)
> +               return ret;
> +
> +       val = HIWORD_UPDATE(RK3288_UOC0_CON3_UTMI_OPMODE_NODRIVING
> +                               | RK3288_UOC0_CON3_UTMI_XCVRSEELCT_FSTRANSC
> +                               | RK3288_UOC0_CON3_UTMI_TERMSEL_FULLSPEED,
> +                           RK3288_UOC0_CON3_UTMI_SUSPENDN
> +                               | RK3288_UOC0_CON3_UTMI_OPMODE_MASK
> +                               | RK3288_UOC0_CON3_UTMI_XCVRSEELCT_MASK
> +                               | RK3288_UOC0_CON3_UTMI_TERMSEL_FULLSPEED);
> +       ret = regmap_write(grf, RK3288_UOC0_CON3, val);
> +       if (ret)
> +               return ret;
> +
> +       val = HIWORD_UPDATE(RK3288_UOC0_CON3_BYPASSSEL
> +                               | RK3288_UOC0_CON3_BYPASSDMEN,
> +                           RK3288_UOC0_CON3_BYPASSSEL
> +                               | RK3288_UOC0_CON3_BYPASSDMEN);
> +       ret = regmap_write(grf, RK3288_UOC0_CON3, val);
> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +static const struct rockchip_uart_data rk3288_uart_data __initconst = {
> +       .grf_compatible = "rockchip,rk3288-grf",
> +       .usb_compatible = "rockchip,rk3288-usb",
> +       .usb_phys_addr = 0xff580000,
> +       .init_uart = rk3288_init_usb_uart,
> +};
> +
> +static const struct of_device_id rockchip_usb_uart_ids[] __initconst = {
> +       { .compatible = "rockchip,rk3288", .data = &rk3288_uart_data },
> +       { }
> +};
> +
> +/*
> + * Find the usb controller using the shared usb-uart-phy in the dts and
> + * disable it.
> + */
> +static int __init
> +rockchip_disable_usb_controller(const struct rockchip_uart_data *data)
> +{
> +       struct device_node *np;
> +
> +       for_each_compatible_node(np, NULL, data->usb_compatible) {
> +               struct property *new_status;
> +               struct resource res;
> +               int ret;
> +
> +               ret = of_address_to_resource(np, 0, &res);
> +               if (ret) {
> +                       pr_err("%s: could not get address of usb controller %s\n",
> +                              __func__, np->full_name);
> +                       continue;
> +               }
> +
> +               /* not the controller we're looking for */
> +               if (res.start != data->usb_phys_addr)
> +                       continue;
> +
> +               pr_debug("%s: disabling usb controller %s\n",
> +                        __func__, np->full_name);
> +
> +               new_status = kzalloc(sizeof(*new_status), GFP_KERNEL);
> +               if (!new_status)
> +                       return -ENOMEM;
> +
> +               new_status->name = kstrdup("status", GFP_KERNEL);
> +               new_status->length = sizeof("disabled");
> +               new_status->value = kstrdup("disabled", GFP_KERNEL);

I understand completly why you need to pass an property allocated
dynamically, however, is it absolutly required to allocate "name" and
"value" dynamically too ? I mean, you could assign const literal
strings directly *BUT* it strongly depends on how properties are freed
by the devicetree subsystem.

> +
> +               return of_update_property(np, new_status);
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id __init *rockchip_usb_uart_data_lookup(void)
> +{
> +       struct device_node *root;
> +       const struct of_device_id *id;
> +
> +       root = of_find_node_by_path("/");
> +       if (!root)
> +               return NULL;
> +
> +       id = of_match_node(rockchip_usb_uart_ids, root);
> +       of_node_put(root);
> +
> +       return id;
> +}
> +
> +static int __init rockchip_init_usb_uart(void)
> +{
> +       const struct of_device_id *match;
> +       const struct rockchip_uart_data *data;
> +       struct regmap *grf;
> +       int ret;
> +
> +       if (!enable_usb_uart)
> +               return 0;
> +
> +       match = rockchip_usb_uart_data_lookup();
> +       if (!match)
> +               return -ENOTSUPP;
> +
> +       pr_debug("%s: using settings for %s\n", __func__, match->compatible);
> +       data = match->data;
> +
> +       grf = syscon_regmap_lookup_by_compatible(data->grf_compatible);
> +       if (IS_ERR(grf)) {
> +               pr_err("%s: could not find GRF syscon\n", __func__);
> +               return PTR_ERR(grf);
> +       }
> +
> +       ret = data->init_uart(data, grf);
> +       if (ret) {
> +               pr_err("%s: could not init usb_uart\n", __func__);
> +               return ret;
> +       }
> +
> +       return rockchip_disable_usb_controller(data);
> +}
> +early_initcall(rockchip_init_usb_uart);
> +
> +static int __init rockchip_usb_uart(char *buf)
> +{
> +       enable_usb_uart = true;
> +       return 0;
> +}
> +early_param("rockchip.usb_uart", rockchip_usb_uart);
> --
> 2.1.4
>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


I agree with Arnd, it would be better to lose nothing. Some users
might have rk3288-based tv box (assuming that the dts is opened...) or
broken uart... yes you can do low levels tasks on a development board,
but that's not a real solution for me. Each device should be
independent, imho.

Regards,
Romain