Re: [PATCH v6 09/18] video: tegra20: dc: fix printing of framebuffer address
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> From: Jonas Schwöbel
>
> Framebuffer address should not be a pointer.
>
> Signed-off-by: Jonas Schwöbel
> Signed-off-by: Svyatoslav Ryhel
> ---
> drivers/video/tegra20/tegra-dc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/tegra20/tegra-dc.c
> b/drivers/video/tegra20/tegra-dc.c
> index a8e32e6893..000a0e02f8 100644
> --- a/drivers/video/tegra20/tegra-dc.c
> +++ b/drivers/video/tegra20/tegra-dc.c
> @@ -429,7 +429,7 @@ static int tegra_lcd_probe(struct udevice *dev)
> uc_priv->xsize = priv->width;
> uc_priv->ysize = priv->height;
> uc_priv->bpix = priv->log2_bpp;
> - debug("LCD frame buffer at %pa, size %x\n", >frame_buffer,
> + debug("LCD frame buffer at %08x, size %x\n", priv->frame_buffer,
%pa is usually treated in a special way. At least it is on Linux, not
sure if the U-Boot printf implementation uses this, too. Looking at
doc/develop/printf.rst, it seems like this is supported, and given that
priv->frame_buffer is fdt_addr_t, %pa seems totally appropriate here.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v6 12/18] video: tegra20: dc: parameterize V- and H-sync polarities
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> Based on Thierry Reding's Linux commit:
>
> 'commit 1716b1891e1de05e2c20ccafa9f58550f3539717
> ("drm/tegra: rgb: Parameterize V- and H-sync polarities")'
>
> Signed-off-by: Svyatoslav Ryhel
> ---
> arch/arm/include/asm/arch-tegra/dc.h | 5 +
> drivers/video/tegra20/tegra-dc.c | 22 --
> 2 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-tegra/dc.h
> b/arch/arm/include/asm/arch-tegra/dc.h
> index 6444af2993..ca3718411a 100644
> --- a/arch/arm/include/asm/arch-tegra/dc.h
> +++ b/arch/arm/include/asm/arch-tegra/dc.h
> @@ -443,6 +443,11 @@ enum win_color_depth_id {
> #define WINDOW_D_SELECT BIT(7)
> #define WINDOW_H_SELECT BIT(8)
>
> +/* DC_COM_PIN_OUTPUT_POLARITY1 0x307 */
> +#define LHS_OUTPUT_POLARITY_LOW BIT(30)
> +#define LVS_OUTPUT_POLARITY_LOW BIT(28)
> +#define LSC0_OUTPUT_POLARITY_LOW BIT(24)
This definition seems to be unused, other than that:
Reviewed-by: Thierry Reding
signature.asc
Description: PGP signature
Re: [PATCH v6 04/18] video: tegra20: dc: pass DC id to internal devices
On Fri Apr 19, 2024 at 6:44 PM CEST, Svyatoslav Ryhel wrote:
> пт, 19 квіт. 2024 р. о 19:38 Thierry Reding пише:
> >
> > On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> > > Tegra SoC has 2 independent display controllers called DC_A and
> > > DC_B, they are handled differently by internal video devices like
> > > DSI and HDMI controllers so it is important for last to know
> > > which display controller is used to properly set up registers.
> > > To achieve this, a pipe field was added to pdata to pass display
> > > controller id to internal Tegra SoC devices.
> > >
> > > Tested-by: Agneli # Toshiba AC100 T20
> > > Tested-by: Robert Eckelmann # ASUS TF101
> > > Tested-by: Andreas Westman Dorcsak # ASUS Grouper E1565
> > > Tested-by: Ion Agorria # HTC One X
> > > Tested-by: Svyatoslav Ryhel # Nvidia Tegratab T114
> > > Signed-off-by: Svyatoslav Ryhel
> > > ---
> > > drivers/video/tegra20/tegra-dc.c | 6 ++
> > > drivers/video/tegra20/tegra-dc.h | 3 +++
> > > 2 files changed, 9 insertions(+)
> > >
> > > diff --git a/drivers/video/tegra20/tegra-dc.c
> > > b/drivers/video/tegra20/tegra-dc.c
> > > index 5d8874f323..0e94e665ef 100644
> > > --- a/drivers/video/tegra20/tegra-dc.c
> > > +++ b/drivers/video/tegra20/tegra-dc.c
> > > @@ -45,6 +45,7 @@ struct tegra_lcd_priv {
> > > unsigned pixel_clock; /* Pixel clock in Hz */
> > > int dc_clk[2]; /* Contains clk and its parent */
> > > bool rotation; /* 180 degree panel turn */
> > > + bool pipe; /* DC controller: 0 for A, 1 for B
> > > */
> >
> > Bool is a poor choice, even if there's only two of them. This is a
> > proper index, so it should be some sort of integer.
> >
> > Also, the device tree bindings for the display controller specify a
> > "nvidia,head" property that can be used to identify these. If you add
> > that to the U-Boot DT you can avoid looking up by name to map this
> > value.
> >
>
> Thanks for pointing to this property. May we apply this patch set as is
> since it is well tested and confirmed to work and I will prepare a follow
> up patches to adjust device tree relations? Would what be ok?
Well, there's a few other things that I think should be addressed, but
if you'd like to keep this one patch as-is and clean this up later, I
guess that's fine.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v6 07/18] video: tegra20: dc: add powergate
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote: > Add powergate use on T114 to complete resetting of DC. > > Signed-off-by: Svyatoslav Ryhel > --- > drivers/video/tegra20/tegra-dc.c | 27 +++ > 1 file changed, 27 insertions(+) Seems correct: Reviewed-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v6 11/18] video: tegra20: dc: clean framebuffer memory block
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote: > From: Jonas Schwöbel > > Fill the framebuffer memory with zeros to avoid visual glitches. > > Signed-off-by: Jonas Schwöbel > Signed-off-by: Svyatoslav Ryhel > --- > drivers/video/tegra20/tegra-dc.c | 5 + > 1 file changed, 5 insertions(+) Makes sense: Reviewed-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v6 15/18] video: tegra20: dsi: add reset support
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> Implement reset use to discard any changes which could have been
> applied to DSI before and can interfere with current configuration.
>
> Tested-by: Ion Agorria # HTC One X
> Tested-by: Svyatoslav Ryhel # Nvidia Tegratab T114
> Signed-off-by: Svyatoslav Ryhel
> ---
> drivers/video/tegra20/tegra-dsi.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/video/tegra20/tegra-dsi.c
> b/drivers/video/tegra20/tegra-dsi.c
> index 25a629535e..fc9ca1310a 100644
> --- a/drivers/video/tegra20/tegra-dsi.c
> +++ b/drivers/video/tegra20/tegra-dsi.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -863,6 +864,7 @@ static int tegra_dsi_bridge_probe(struct udevice *dev)
> struct tegra_dsi_priv *priv = dev_get_priv(dev);
> struct mipi_dsi_device *device = >device;
> struct mipi_dsi_panel_plat *mipi_plat;
> + struct reset_ctl reset_ctl;
> int ret;
>
> priv->version = dev_get_driver_data(dev);
> @@ -876,6 +878,13 @@ static int tegra_dsi_bridge_probe(struct udevice *dev)
> priv->video_fifo_depth = 480;
> priv->host_fifo_depth = 64;
>
> + ret = reset_get_by_name(dev, "dsi", _ctl);
> + if (ret) {
> + log_debug("%s: reset_get_by_name() failed: %d\n",
> + __func__, ret);
> + return ret;
> + }
> +
> ret = uclass_get_device_by_phandle(UCLASS_REGULATOR, dev,
> "avdd-dsi-csi-supply", >avdd);
> if (ret)
> @@ -914,12 +923,17 @@ static int tegra_dsi_bridge_probe(struct udevice *dev)
>
> tegra_dsi_get_format(device->format, >format);
>
> + reset_assert(_ctl);
> +
> ret = regulator_set_enable_if_allowed(priv->avdd, true);
> if (ret && ret != -ENOSYS)
> return ret;
>
> tegra_dsi_init_clocks(dev);
>
> + mdelay(2);
> + reset_deassert(_ctl);
> +
> return 0;
> }
Looks like tegra_dsi_init_clocks() already does the mdelay() and
reset_deassert()? Or perhaps I'm again looking at things in the wrong
order?
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v6 05/18] video: tegra20: dc: add PLLD2 parent support
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote: > T30+ SOC have second PLLD - PLLD2 which can be actively used by > DC and act as main DISP1/2 clock parent. > > Tested-by: Agneli # Toshiba AC100 T20 > Tested-by: Robert Eckelmann # ASUS TF101 > Tested-by: Andreas Westman Dorcsak # ASUS Grouper E1565 > Tested-by: Ion Agorria # HTC One X > Tested-by: Svyatoslav Ryhel # Nvidia Tegratab T114 > Signed-off-by: Svyatoslav Ryhel > --- > drivers/video/tegra20/tegra-dc.c | 6 ++ > 1 file changed, 6 insertions(+) Reviewed-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v6 08/18] video: tegra20: dc: configure behavior if PLLD/D2 is used
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote: > If DISP1 is a PLLD/D2 child, it cannot go over 370MHz. The cause > of this is not quite clear. This can be overcomed by further > halving the PLLD/D2 if the target parent rate is over 800MHz. > This way DISP1 and DSI clocks will have the same frequency. The > shift divider in this case has to be calculated from the > original PLLD/D2 frequency and is passed from the DSI driver. > > Tested-by: Andreas Westman Dorcsak # ASUS Grouper E1565 > Tested-by: Ion Agorria # HTC One X > Tested-by: Svyatoslav Ryhel # Nvidia Tegratab T114 > Tested-by: Jonas Schwöbel # Microsoft Surface 2 > Signed-off-by: Jonas Schwöbel > Signed-off-by: Svyatoslav Ryhel > --- > drivers/video/tegra20/tegra-dc.c | 34 +++ > drivers/video/tegra20/tegra-dc.h | 1 + > drivers/video/tegra20/tegra-dsi.c | 14 + > 3 files changed, 36 insertions(+), 13 deletions(-) I'm not very familiar with these clocks, but seeing that this was extensively tested, I guess this is okay, so: Acked-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v6 06/18] video: tegra20: dc: add reset support
On Fri Apr 19, 2024 at 6:37 PM CEST, Svyatoslav Ryhel wrote:
> пт, 19 квіт. 2024 р. о 19:05 Thierry Reding пише:
> >
> > On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> > > Implement reset use to discard any changes which could have been
> > > applied to DC before and can interfere with current configuration.
> > >
> > > Tested-by: Agneli # Toshiba AC100 T20
> > > Tested-by: Robert Eckelmann # ASUS TF101
> > > Tested-by: Andreas Westman Dorcsak # ASUS Grouper E1565
> > > Tested-by: Ion Agorria # HTC One X
> > > Tested-by: Svyatoslav Ryhel # Nvidia Tegratab T114
> > > Signed-off-by: Svyatoslav Ryhel
> > > ---
> > > drivers/video/tegra20/tegra-dc.c | 17 +
> > > 1 file changed, 17 insertions(+)
> > >
> > > diff --git a/drivers/video/tegra20/tegra-dc.c
> > > b/drivers/video/tegra20/tegra-dc.c
> > > index 56a23b3c97..35abb6fe46 100644
> > > --- a/drivers/video/tegra20/tegra-dc.c
> > > +++ b/drivers/video/tegra20/tegra-dc.c
> > > @@ -10,7 +10,9 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -342,6 +344,7 @@ static int tegra_lcd_probe(struct udevice *dev)
> > > struct video_uc_plat *plat = dev_get_uclass_plat(dev);
> > > struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> > > struct tegra_lcd_priv *priv = dev_get_priv(dev);
> > > + struct reset_ctl reset_ctl;
> > > int ret;
> > >
> > > /* Initialize the Tegra display controller */
> > > @@ -349,6 +352,20 @@ static int tegra_lcd_probe(struct udevice *dev)
> > > funcmux_select(PERIPH_ID_DISP1, FUNCMUX_DEFAULT);
> > > #endif
> > >
> > > + ret = reset_get_by_name(dev, "dc", _ctl);
> > > + if (ret) {
> > > + log_err("reset_get_by_name() failed: %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + clock_disable(priv->dc_clk[0]);
> > > +
> > > + /* Reset everything set before */
> > > + reset_assert(_ctl);
> > > + mdelay(4);
> > > + reset_deassert(_ctl);
> > > + mdelay(4);
> >
> > Are you sure this works as intended? It's been a long time since I
> > worked on this, but I seem to recall that most of these resets are
> > actually synchronous, so in order for them to do what they're supposed
> > to the clock needs to be kept running.
> >
> > The Linux driver certainly does this differently.
>
> You have point, but I have tried to mostly adapt Linux tegra dc driver,
> which has same logic in probe. Maybe I have not understood it properly.
> Testing on T20, T30 and T114 passed without issues so far.
Maybe look again. What it does is (basically):
clock_enable();
mdelay(4);
reset_assert();
mdelay(4);
clock_disable();
That should ensure that it's completely reset at that point. Now before
any subsequent register accesses happen it will do this:
clock_enable();
reset_deassert();
to take it out of reset again. Perhaps that's something you want to keep
doing in probe() in U-Boot. In that case maybe you want something like
this instead:
clock_enable();
mdelay(4);
reset_assert();
mdelay(4);
reset_deassert();
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v6 16/18] video: tegra20: dsi: remove pre-configuration
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote: > From: Jonas Schwöbel > > Configuration for DC driver command mode is not required for > every panel. Removed. > > Tested-by: Ion Agorria # HTC One X > Tested-by: Svyatoslav Ryhel # Nvidia Tegratab T114 > Signed-off-by: Jonas Schwöbel > Signed-off-by: Svyatoslav Ryhel > --- > drivers/video/tegra20/tegra-dsi.c | 2 -- > 1 file changed, 2 deletions(-) Yeah, this seems completely unnecessary. Reviewed-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v6 04/18] video: tegra20: dc: pass DC id to internal devices
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> Tegra SoC has 2 independent display controllers called DC_A and
> DC_B, they are handled differently by internal video devices like
> DSI and HDMI controllers so it is important for last to know
> which display controller is used to properly set up registers.
> To achieve this, a pipe field was added to pdata to pass display
> controller id to internal Tegra SoC devices.
>
> Tested-by: Agneli # Toshiba AC100 T20
> Tested-by: Robert Eckelmann # ASUS TF101
> Tested-by: Andreas Westman Dorcsak # ASUS Grouper E1565
> Tested-by: Ion Agorria # HTC One X
> Tested-by: Svyatoslav Ryhel # Nvidia Tegratab T114
> Signed-off-by: Svyatoslav Ryhel
> ---
> drivers/video/tegra20/tegra-dc.c | 6 ++
> drivers/video/tegra20/tegra-dc.h | 3 +++
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/video/tegra20/tegra-dc.c
> b/drivers/video/tegra20/tegra-dc.c
> index 5d8874f323..0e94e665ef 100644
> --- a/drivers/video/tegra20/tegra-dc.c
> +++ b/drivers/video/tegra20/tegra-dc.c
> @@ -45,6 +45,7 @@ struct tegra_lcd_priv {
> unsigned pixel_clock; /* Pixel clock in Hz */
> int dc_clk[2]; /* Contains clk and its parent */
> bool rotation; /* 180 degree panel turn */
> + bool pipe; /* DC controller: 0 for A, 1 for B */
Bool is a poor choice, even if there's only two of them. This is a
proper index, so it should be some sort of integer.
Also, the device tree bindings for the display controller specify a
"nvidia,head" property that can be used to identify these. If you add
that to the U-Boot DT you can avoid looking up by name to map this
value.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v6 10/18] video: tegra20: dc: enable backlight after DC is configured
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote: > From: Jonas Schwöbel > > The goal of panel_set_backlight() is to enable backlight. Hence, > it should be called at the probe end. > > Signed-off-by: Jonas Schwöbel > Signed-off-by: Svyatoslav Ryhel > --- > drivers/video/tegra20/tegra-dc.c | 8 +--- > 1 file changed, 1 insertion(+), 7 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v6 01/18] video: tegra20: dc: diverge DC per-SOC
On Fri Apr 19, 2024 at 6:26 PM CEST, Thierry Reding wrote:
> On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> [...]
> > diff --git a/arch/arm/include/asm/arch-tegra114/display.h
> > b/arch/arm/include/asm/arch-tegra114/display.h
> > new file mode 100644
> > index 00..9411525799
> > --- /dev/null
> > +++ b/arch/arm/include/asm/arch-tegra114/display.h
> > @@ -0,0 +1,28 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * (C) Copyright 2010
> > + * NVIDIA Corporation
> > + */
> > +
> > +#ifndef __ASM_ARCH_TEGRA_DISPLAY_H
> > +#define __ASM_ARCH_TEGRA_DISPLAY_H
> > +
> > +#include
> > +
> > +/* This holds information about a window which can be displayed */
> > +struct disp_ctl_win {
> > + enum win_color_depth_id fmt;/* Color depth/format */
> > + unsigned intbpp;/* Bits per pixel */
> > + phys_addr_t phys_addr; /* Physical address in memory */
> > + unsigned intx; /* Horizontal address offset (bytes) */
> > + unsigned inty; /* Veritical address offset (bytes) */
> > + unsigned intw; /* Width of source window */
> > + unsigned inth; /* Height of source window */
> > + unsigned intstride; /* Number of bytes per line */
> > + unsigned intout_x; /* Left edge of output window (col) */
> > + unsigned intout_y; /* Top edge of output window (row) */
> > + unsigned intout_w; /* Width of output window in pixels */
> > + unsigned intout_h; /* Height of output window in pixels */
> > +};
> > +
> > +#endif /*__ASM_ARCH_TEGRA_DISPLAY_H*/
>
> One of the earlier patches in the series gets rid of this per-SoC header
> file in favor of a common one. Did this end up here by mistake? It
> doesn't seem to be used.
Nevermind, my MUA sorted these patches weirdly, so it appeared as if
this was later in the series than it actually was.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v6 02/18] video: tegra20: dc: fix image shift on rotated panels
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote: > Subtracting 1 from x and y fixes image shifting on rotated > panels. > > Tested-by: Robert Eckelmann # ASUS Grouper E1565 > Signed-off-by: Svyatoslav Ryhel > --- > drivers/video/tegra20/tegra-dc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Yeah, looks like we have that same fixup in Linux: Reviewed-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v6 17/18] video: tegra20: dsi: set correct fifo depth
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote: > From: Jonas Schwöbel > > According to Thierry Reding's commit in the linux kernel > > 976cebc35bed0456a42bf96073a26f251d23b264 > "drm/tegra: dsi: Make FIFO depths host parameters" > > correct depth of the video FIFO is 1920 *words* no *bytes* > > Tested-by: Ion Agorria # HTC One X > Tested-by: Svyatoslav Ryhel # Nvidia Tegratab T114 > Signed-off-by: Jonas Schwöbel > Signed-off-by: Svyatoslav Ryhel > --- > drivers/video/tegra20/tegra-dsi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v6 14/18] video: tegra20: dsi: add T114 support
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote: > Existing Tegra DSI driver mostly fits T114 apart MIPI calibration > which on T114 has dedicated driver. To resolve this MIPI calibration > logic was split for pre-T114 and T114+ devices. > > Tested-by: Ion Agorria # HTC One X > Tested-by: Svyatoslav Ryhel # Nvidia Tegratab T114 > Signed-off-by: Svyatoslav Ryhel > --- > drivers/video/tegra20/tegra-dsi.c | 78 ++- > .../video/tegra20/tegra-dsi.h | 24 +- > 2 files changed, 96 insertions(+), 6 deletions(-) > rename arch/arm/include/asm/arch-tegra30/dsi.h => > drivers/video/tegra20/tegra-dsi.h (90%) Reviewed-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v6 01/18] video: tegra20: dc: diverge DC per-SOC
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
[...]
> diff --git a/arch/arm/include/asm/arch-tegra114/display.h
> b/arch/arm/include/asm/arch-tegra114/display.h
> new file mode 100644
> index 00..9411525799
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-tegra114/display.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * (C) Copyright 2010
> + * NVIDIA Corporation
> + */
> +
> +#ifndef __ASM_ARCH_TEGRA_DISPLAY_H
> +#define __ASM_ARCH_TEGRA_DISPLAY_H
> +
> +#include
> +
> +/* This holds information about a window which can be displayed */
> +struct disp_ctl_win {
> + enum win_color_depth_id fmt;/* Color depth/format */
> + unsigned intbpp;/* Bits per pixel */
> + phys_addr_t phys_addr; /* Physical address in memory */
> + unsigned intx; /* Horizontal address offset (bytes) */
> + unsigned inty; /* Veritical address offset (bytes) */
> + unsigned intw; /* Width of source window */
> + unsigned inth; /* Height of source window */
> + unsigned intstride; /* Number of bytes per line */
> + unsigned intout_x; /* Left edge of output window (col) */
> + unsigned intout_y; /* Top edge of output window (row) */
> + unsigned intout_w; /* Width of output window in pixels */
> + unsigned intout_h; /* Height of output window in pixels */
> +};
> +
> +#endif /*__ASM_ARCH_TEGRA_DISPLAY_H*/
One of the earlier patches in the series gets rid of this per-SoC header
file in favor of a common one. Did this end up here by mistake? It
doesn't seem to be used.
> diff --git a/drivers/video/tegra20/tegra-dc.c
> b/drivers/video/tegra20/tegra-dc.c
> index f53ad46397..7605e77bc1 100644
> --- a/drivers/video/tegra20/tegra-dc.c
> +++ b/drivers/video/tegra20/tegra-dc.c
> @@ -3,7 +3,6 @@
> * Copyright (c) 2011 The Chromium OS Authors.
> */
>
> -#include
> #include
> #include
> #include
> @@ -23,10 +22,15 @@
> #include
> #include
> #include
> -#include
>
> DECLARE_GLOBAL_DATA_PTR;
>
> +/* Holder of Tegra per-SOC DC differences */
> +struct tegra_dc_soc_info {
> + bool has_timer;
> + bool has_rgb;
> +};
> +
> /* Information about the display controller */
> struct tegra_lcd_priv {
> int width; /* width in pixels */
> @@ -35,6 +39,7 @@ struct tegra_lcd_priv {
> struct display_timing timing;
> struct udevice *panel;
> struct dc_ctlr *dc; /* Display controller regmap */
> + const struct tegra_dc_soc_info *soc;
> fdt_addr_t frame_buffer;/* Address of frame buffer */
> unsigned pixel_clock; /* Pixel clock in Hz */
> int dc_clk[2]; /* Contains clk and its parent */
> @@ -43,8 +48,8 @@ struct tegra_lcd_priv {
>
> enum {
> /* Maximum LCD size we support */
> - LCD_MAX_WIDTH = 1920,
> - LCD_MAX_HEIGHT = 1200,
> + LCD_MAX_WIDTH = 2560,
> + LCD_MAX_HEIGHT = 1600,
> LCD_MAX_LOG2_BPP= VIDEO_BPP16,
> };
>
> @@ -110,9 +115,9 @@ static void update_window(struct tegra_lcd_priv *priv,
> writel(val, >cmd.state_ctrl);
> }
>
> -static int update_display_mode(struct dc_disp_reg *disp,
> -struct tegra_lcd_priv *priv)
> +static int update_display_mode(struct tegra_lcd_priv *priv)
> {
> + struct dc_disp_reg *disp = >dc->disp;
> struct display_timing *dt = >timing;
> unsigned long val;
> unsigned long rate;
> @@ -128,14 +133,16 @@ static int update_display_mode(struct dc_disp_reg *disp,
> >front_porch);
> writel(dt->hactive.typ | (dt->vactive.typ << 16), >disp_active);
>
> - val = DE_SELECT_ACTIVE << DE_SELECT_SHIFT;
> - val |= DE_CONTROL_NORMAL << DE_CONTROL_SHIFT;
> - writel(val, >data_enable_opt);
> + if (priv->soc->has_rgb) {
> + val = DE_SELECT_ACTIVE << DE_SELECT_SHIFT;
> + val |= DE_CONTROL_NORMAL << DE_CONTROL_SHIFT;
> + writel(val, >data_enable_opt);
>
> - val = DATA_FORMAT_DF1P1C << DATA_FORMAT_SHIFT;
> - val |= DATA_ALIGNMENT_MSB << DATA_ALIGNMENT_SHIFT;
> - val |= DATA_ORDER_RED_BLUE << DATA_ORDER_SHIFT;
> - writel(val, >disp_interface_ctrl);
> + val = DATA_FORMAT_DF1P1C << DATA_FORMAT_SHIFT;
> + val |= DATA_ALIGNMENT_MSB << DATA_ALIGNMENT_SHIFT;
> + val |= DATA_ORDER_RED_BLUE << DATA_ORDER_SHIFT;
> + writel(val, >disp_interface_ctrl);
> + }
>
> /*
>* The pixel clock divider is in 7.1 format (where the bottom bit
> @@ -147,7 +154,8 @@ static int update_display_mode(struct dc_disp_reg *disp,
> div = ((rate * 2 + priv->pixel_clock / 2) / priv->pixel_clock) - 2;
> debug("Display clock %lu, divider %lu\n", rate, div);
>
> -
Re: [PATCH v6 13/18] video: tegra20: add MIPI calibration driver
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote: > Dedicated MIPI calibration driver is used on T114 and newer. Before > T114 MIPI calibration registers were part of VI and CSI. > > Tested-by: Svyatoslav Ryhel # Nvidia Tegratab T114 > Signed-off-by: Svyatoslav Ryhel > --- > drivers/video/tegra20/Makefile | 2 +- > drivers/video/tegra20/tegra-mipi.c | 188 + > 2 files changed, 189 insertions(+), 1 deletion(-) > create mode 100644 drivers/video/tegra20/tegra-mipi.c This looks like an adequate copy of what we have in the Linux kernel. It's slightly suboptimal because most of the register values are hard- coded for Tegra114, but I guess that can always be improved later on. Reviewed-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v6 06/18] video: tegra20: dc: add reset support
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> Implement reset use to discard any changes which could have been
> applied to DC before and can interfere with current configuration.
>
> Tested-by: Agneli # Toshiba AC100 T20
> Tested-by: Robert Eckelmann # ASUS TF101
> Tested-by: Andreas Westman Dorcsak # ASUS Grouper E1565
> Tested-by: Ion Agorria # HTC One X
> Tested-by: Svyatoslav Ryhel # Nvidia Tegratab T114
> Signed-off-by: Svyatoslav Ryhel
> ---
> drivers/video/tegra20/tegra-dc.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/video/tegra20/tegra-dc.c
> b/drivers/video/tegra20/tegra-dc.c
> index 56a23b3c97..35abb6fe46 100644
> --- a/drivers/video/tegra20/tegra-dc.c
> +++ b/drivers/video/tegra20/tegra-dc.c
> @@ -10,7 +10,9 @@
> #include
> #include
> #include
> +#include
> #include
> +#include
> #include
> #include
> #include
> @@ -342,6 +344,7 @@ static int tegra_lcd_probe(struct udevice *dev)
> struct video_uc_plat *plat = dev_get_uclass_plat(dev);
> struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> struct tegra_lcd_priv *priv = dev_get_priv(dev);
> + struct reset_ctl reset_ctl;
> int ret;
>
> /* Initialize the Tegra display controller */
> @@ -349,6 +352,20 @@ static int tegra_lcd_probe(struct udevice *dev)
> funcmux_select(PERIPH_ID_DISP1, FUNCMUX_DEFAULT);
> #endif
>
> + ret = reset_get_by_name(dev, "dc", _ctl);
> + if (ret) {
> + log_err("reset_get_by_name() failed: %d\n", ret);
> + return ret;
> + }
> +
> + clock_disable(priv->dc_clk[0]);
> +
> + /* Reset everything set before */
> + reset_assert(_ctl);
> + mdelay(4);
> + reset_deassert(_ctl);
> + mdelay(4);
Are you sure this works as intended? It's been a long time since I
worked on this, but I seem to recall that most of these resets are
actually synchronous, so in order for them to do what they're supposed
to the clock needs to be kept running.
The Linux driver certainly does this differently.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v6 18/18] video: tegra20: dsi: use set_backlight for backlight only
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote: > From: Jonas Schwöbel > > Shift the backlight set further to prevent visual glitches on > panel init. > > Signed-off-by: Jonas Schwöbel > Signed-off-by: Svyatoslav Ryhel > --- > drivers/video/tegra20/tegra-dsi.c | 10 -- > 1 file changed, 4 insertions(+), 6 deletions(-) I vaguely recall that some devices may have had the panel and the backlight hooked up to the same regulator or enable GPIO, so calling panel_set_backlight() too late may cause the DSI panel to malfunction. That said, I'm not familiar with panel_set_backlight(), so perhaps it always only sets the brightness and the power may already be on earlier? Ah... nevermind... I see that panel_enable_backlight() is called prior to panel_set_backlight(), so this looks like it should be fine. Reviewed-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v6 03/18] video: tegra20: consolidate DC header
On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote: > Consolidate HD headers and place the result into video/tegra20 Was this supposed to be "DC" headers like in the subject? Other than that this makes sense, so: Reviewed-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v1 0/8] Convert Tegra pinctrl to DM
On Wed, Dec 13, 2023 at 11:46:58AM +0200, Svyatoslav Ryhel wrote: > > > 9 грудня 2023 р. 16:14:06 GMT+02:00, Tom Rini > написав(-ла): > >On Mon, Dec 04, 2023 at 04:45:38PM +0200, Svyatoslav Ryhel wrote: > >> пн, 4 груд. 2023 р. о 12:26 Thierry Reding пише: > >> > > >> > On Mon, Dec 04, 2023 at 10:20:46AM +0200, Svyatoslav Ryhel wrote: > >> > > Create a DM supported wrapper arround existing Tegra pinmux logic. > >> > > This implementation is backwards compatible with all existing board > >> > > setups and early stages of setup. All new boards must be device tree > >> > > based. Linux dts pinmux nodes are fully compatible with this driver. > >> > > > >> > > As an example I have converted recently merged T30 boards to this > >> > > layout (I have those devices and was able to test them properly) and > >> > > Paz00 T20 board which was tested as well by owner. > >> > > > >> > > Svyatoslav Ryhel (8): > >> > > ARM: mach-tegra: rearrange SPL configs > >> > > drivers: pinctrl: create Tegra DM pinctrl driver > >> > > drivers: pinctrl: tegra: incorporate existing code > >> > > board: asus: grouper: switch to DM pinmux > >> > > board: lg: x3-t30: switch to DM pinmux > >> > > board: asus: transformer: switch to DM pinmux > >> > > board: htc: endeavoru: switch to DM pinmux > >> > > board: compal: paz00: clean up the board > >> > > > >> > > arch/arm/dts/tegra20-paz00.dts|4 +- > >> > > arch/arm/dts/tegra30-asus-grouper-common.dtsi | 712 ++ > >> > > .../dts/tegra30-asus-nexus7-grouper-E1565.dts | 113 ++ > >> > > .../dts/tegra30-asus-nexus7-grouper-PM269.dts | 113 ++ > >> > > .../dts/tegra30-asus-nexus7-tilapia-E1565.dts | 149 +++ > >> > > arch/arm/dts/tegra30-asus-p1801-t.dts | 982 ++ > >> > > arch/arm/dts/tegra30-asus-tf201.dts | 45 + > >> > > arch/arm/dts/tegra30-asus-tf300t.dts | 45 + > >> > > arch/arm/dts/tegra30-asus-tf300tg.dts | 128 ++ > >> > > arch/arm/dts/tegra30-asus-tf300tl.dts | 163 +++ > >> > > arch/arm/dts/tegra30-asus-tf600t.dts | 889 + > >> > > arch/arm/dts/tegra30-asus-tf700t.dts | 53 + > >> > > arch/arm/dts/tegra30-asus-transformer.dtsi| 984 ++ > >> > > arch/arm/dts/tegra30-htc-endeavoru.dts| 1147 + > >> > > arch/arm/dts/tegra30-lg-p880.dts | 90 ++ > >> > > arch/arm/dts/tegra30-lg-p895.dts | 93 ++ > >> > > arch/arm/dts/tegra30-lg-x3.dtsi | 845 > >> > > arch/arm/include/asm/arch-tegra114/pinmux.h | 303 + > >> > > arch/arm/include/asm/arch-tegra124/pinmux.h | 327 + > >> > > arch/arm/include/asm/arch-tegra20/pinmux.h| 291 + > >> > > arch/arm/include/asm/arch-tegra210/pinmux.h | 394 ++ > >> > > arch/arm/include/asm/arch-tegra30/pinmux.h| 381 ++ > >> > > arch/arm/mach-tegra/Kconfig | 16 +- > >> > > arch/arm/mach-tegra/Makefile |1 - > >> > > arch/arm/mach-tegra/board.c |6 +- > >> > > arch/arm/mach-tegra/board2.c |2 +- > >> > > arch/arm/mach-tegra/tegra114/Makefile |2 +- > >> > > arch/arm/mach-tegra/tegra124/Makefile |2 - > >> > > arch/arm/mach-tegra/tegra20/Makefile |2 +- > >> > > arch/arm/mach-tegra/tegra210/Makefile |1 - > >> > > arch/arm/mach-tegra/tegra30/Makefile |2 +- > >> > > board/asus/grouper/grouper.c | 18 - > >> > > board/asus/grouper/pinmux-config-grouper.h| 362 -- > >> > > .../pinmux-config-transformer.h | 365 -- > >> > > board/asus/transformer-t30/transformer-t30.c | 23 - > >> > > board/compal/paz00/Makefile |8 - > >> > > board/compal/paz00/paz00.c| 53 - > >> > > board/htc/endeavoru/endeavoru.c | 14 - > >> > > board/htc/endeavoru/pinmux-config-endeavoru.h | 362 -- > >> > > board/lg/x3-t30/Kconfig
Re: [PATCH v1 0/5] Convert recently merged T30 boards to use DM PMIC
On Wed, Dec 13, 2023 at 11:42:45AM +0200, Svyatoslav Ryhel wrote: > > > 15 листопада 2023 р. 21:11:49 GMT+02:00, Tom Rini > написав(-ла): > >On Wed, Nov 15, 2023 at 04:51:08PM +0100, Thierry Reding wrote: > >> On Mon, Nov 06, 2023 at 04:04:07PM -0500, Tom Rini wrote: > >> > On Mon, Nov 06, 2023 at 02:11:16PM +, Peter Robinson wrote: > >> > > On Mon, Nov 6, 2023 at 1:28 PM Svyatoslav Ryhel > >> > > wrote: > >> > > > > >> > > > пн, 6 лист. 2023 р. о 15:13 Peter Robinson > >> > > > пише: > >> > > > > > >> > > > > On Mon, Nov 6, 2023 at 11:58 AM Svyatoslav Ryhel > >> > > > > wrote: > >> > > > > > > >> > > > > > пн, 6 лист. 2023 р. о 13:46 Peter Robinson > >> > > > > > пише: > >> > > > > > > > >> > > > > > > Hi Svyatoslav, > >> > > > > > > > >> > > > > > > > Since the proposed PMIC patches have been accepted, I see > >> > > > > > > > the need > >> > > > > > > > to convert boards which I maintain to use DM drivers instead > >> > > > > > > > of board hacks. > >> > > > > > > > > >> > > > > > > > Svyatoslav Ryhel (5): > >> > > > > > > > board: lg-x3: convert LG Optimus 4X and Vu to use DM PMIC > >> > > > > > > > board: endeavoru: convert HTC One X to use DM PMIC > >> > > > > > > > >> > > > > > > Is there a reason why the two above devices don't appear to > >> > > > > > > have their > >> > > > > > > .dts files in the upstream kernel? > >> > > > > > > > >> > > > > > > >> > > > > > Yes, there is a reason. Linux maintainers treat submitters as > >> > > > > > existential enemies or as dirt at least. I was trying to work > >> > > > > > with > >> > > > > > linux but I have no desire to spend any time to upstream > >> > > > > > endeavoru or > >> > > > > > lg_x3. > >> > > > > > >> > > > > The usual policy for acceptance into U-Boot is to have upstream > >> > > > > review > >> > > > > in the kernel first. > >> > > > > > >> > > > > >> > > > May you point to a policy which clearly and explicitly states this as > >> > > > a mandatory condition? > >> > > > >> > > There have been a number of devices rejected in the past until their > >> > > DT are upstream but I'll leave Tom, who I've explicitly added on cc:, > >> > > to clarify the exact policy. > >> > > >> > Well, here is where it's tricky. I brought this up for one of the > >> > Broadcom MIPS platforms a week or two back, and Linus Walleij's point > >> > (and I'm paraphrasing) is there's not really an upstream for it to go. > >> > > >> > What we cannot have is device tree bindings[1] that aren't upstream or > >> > worse yet conflict with the official bindings. > >> > > >> > So the general way to resolve that is have device tree file be drop-in > >> > from the linux kernel, and what additions we must have be done via > >> > -u-boot.dtsi files. And in turn, some SoCs are better about keeping in > >> > sync with the kernel than other SoCs are. > >> > > >> > Now, upstream being actively hostile to dts files, especially for older > >> > platforms? That's unfortunate. So long as we aren't violating the rules > >> > about bindings, the intention is that we don't have device trees that > >> > are either (a) massively out of sync with the kernel[2] or (b) kept > >> > intentionally mismatched from the kernel. > >> > > >> > -- > >> > Tom > >> > > >> > [1]: There are both examples like binman that Simon is working on at > >> > least but this is more exception than intentional rule. > >> > [2]: Per our other conversions, I know the tegra ones are in this > >> > unfortunate state in general > >> > >> On the Tegra side we've been fairly lax ab
Re: [PATCH v1 0/5] Convert recently merged T30 boards to use DM PMIC
On Wed, Nov 15, 2023 at 02:11:49PM -0500, Tom Rini wrote: > On Wed, Nov 15, 2023 at 04:51:08PM +0100, Thierry Reding wrote: > > On Mon, Nov 06, 2023 at 04:04:07PM -0500, Tom Rini wrote: > > > On Mon, Nov 06, 2023 at 02:11:16PM +, Peter Robinson wrote: > > > > On Mon, Nov 6, 2023 at 1:28 PM Svyatoslav Ryhel > > > > wrote: > > > > > > > > > > пн, 6 лист. 2023 р. о 15:13 Peter Robinson > > > > > пише: > > > > > > > > > > > > On Mon, Nov 6, 2023 at 11:58 AM Svyatoslav Ryhel > > > > > > wrote: > > > > > > > > > > > > > > пн, 6 лист. 2023 р. о 13:46 Peter Robinson > > > > > > > пише: > > > > > > > > > > > > > > > > Hi Svyatoslav, > > > > > > > > > > > > > > > > > Since the proposed PMIC patches have been accepted, I see the > > > > > > > > > need > > > > > > > > > to convert boards which I maintain to use DM drivers instead > > > > > > > > > of board hacks. > > > > > > > > > > > > > > > > > > Svyatoslav Ryhel (5): > > > > > > > > > board: lg-x3: convert LG Optimus 4X and Vu to use DM PMIC > > > > > > > > > board: endeavoru: convert HTC One X to use DM PMIC > > > > > > > > > > > > > > > > Is there a reason why the two above devices don't appear to > > > > > > > > have their > > > > > > > > .dts files in the upstream kernel? > > > > > > > > > > > > > > > > > > > > > > Yes, there is a reason. Linux maintainers treat submitters as > > > > > > > existential enemies or as dirt at least. I was trying to work with > > > > > > > linux but I have no desire to spend any time to upstream > > > > > > > endeavoru or > > > > > > > lg_x3. > > > > > > > > > > > > The usual policy for acceptance into U-Boot is to have upstream > > > > > > review > > > > > > in the kernel first. > > > > > > > > > > > > > > > > May you point to a policy which clearly and explicitly states this as > > > > > a mandatory condition? > > > > > > > > There have been a number of devices rejected in the past until their > > > > DT are upstream but I'll leave Tom, who I've explicitly added on cc:, > > > > to clarify the exact policy. > > > > > > Well, here is where it's tricky. I brought this up for one of the > > > Broadcom MIPS platforms a week or two back, and Linus Walleij's point > > > (and I'm paraphrasing) is there's not really an upstream for it to go. > > > > > > What we cannot have is device tree bindings[1] that aren't upstream or > > > worse yet conflict with the official bindings. > > > > > > So the general way to resolve that is have device tree file be drop-in > > > from the linux kernel, and what additions we must have be done via > > > -u-boot.dtsi files. And in turn, some SoCs are better about keeping in > > > sync with the kernel than other SoCs are. > > > > > > Now, upstream being actively hostile to dts files, especially for older > > > platforms? That's unfortunate. So long as we aren't violating the rules > > > about bindings, the intention is that we don't have device trees that > > > are either (a) massively out of sync with the kernel[2] or (b) kept > > > intentionally mismatched from the kernel. > > > > > > -- > > > Tom > > > > > > [1]: There are both examples like binman that Simon is working on at > > > least but this is more exception than intentional rule. > > > [2]: Per our other conversions, I know the tegra ones are in this > > > unfortunate state in general > > > > On the Tegra side we've been fairly lax about the device trees in > > U-Boot, I suppose. The assumption had always been that U-Boot would load > > an external DTB and pass it to the kernel on boot, so keeping them both > > in sync was never a high priority. > > > > U-Boot does only a very tiny amount of what Linux does, so dropping in > > the kernel DTB always seemed a bit overkill. > > > > In either case, if this is problematic, it's som
Re: [PATCH v1 0/8] Convert Tegra pinctrl to DM
On Mon, Dec 04, 2023 at 10:20:46AM +0200, Svyatoslav Ryhel wrote: > Create a DM supported wrapper arround existing Tegra pinmux logic. > This implementation is backwards compatible with all existing board > setups and early stages of setup. All new boards must be device tree > based. Linux dts pinmux nodes are fully compatible with this driver. > > As an example I have converted recently merged T30 boards to this > layout (I have those devices and was able to test them properly) and > Paz00 T20 board which was tested as well by owner. > > Svyatoslav Ryhel (8): > ARM: mach-tegra: rearrange SPL configs > drivers: pinctrl: create Tegra DM pinctrl driver > drivers: pinctrl: tegra: incorporate existing code > board: asus: grouper: switch to DM pinmux > board: lg: x3-t30: switch to DM pinmux > board: asus: transformer: switch to DM pinmux > board: htc: endeavoru: switch to DM pinmux > board: compal: paz00: clean up the board > > arch/arm/dts/tegra20-paz00.dts|4 +- > arch/arm/dts/tegra30-asus-grouper-common.dtsi | 712 ++ > .../dts/tegra30-asus-nexus7-grouper-E1565.dts | 113 ++ > .../dts/tegra30-asus-nexus7-grouper-PM269.dts | 113 ++ > .../dts/tegra30-asus-nexus7-tilapia-E1565.dts | 149 +++ > arch/arm/dts/tegra30-asus-p1801-t.dts | 982 ++ > arch/arm/dts/tegra30-asus-tf201.dts | 45 + > arch/arm/dts/tegra30-asus-tf300t.dts | 45 + > arch/arm/dts/tegra30-asus-tf300tg.dts | 128 ++ > arch/arm/dts/tegra30-asus-tf300tl.dts | 163 +++ > arch/arm/dts/tegra30-asus-tf600t.dts | 889 + > arch/arm/dts/tegra30-asus-tf700t.dts | 53 + > arch/arm/dts/tegra30-asus-transformer.dtsi| 984 ++ > arch/arm/dts/tegra30-htc-endeavoru.dts| 1147 + > arch/arm/dts/tegra30-lg-p880.dts | 90 ++ > arch/arm/dts/tegra30-lg-p895.dts | 93 ++ > arch/arm/dts/tegra30-lg-x3.dtsi | 845 > arch/arm/include/asm/arch-tegra114/pinmux.h | 303 + > arch/arm/include/asm/arch-tegra124/pinmux.h | 327 + > arch/arm/include/asm/arch-tegra20/pinmux.h| 291 + > arch/arm/include/asm/arch-tegra210/pinmux.h | 394 ++ > arch/arm/include/asm/arch-tegra30/pinmux.h| 381 ++ > arch/arm/mach-tegra/Kconfig | 16 +- > arch/arm/mach-tegra/Makefile |1 - > arch/arm/mach-tegra/board.c |6 +- > arch/arm/mach-tegra/board2.c |2 +- > arch/arm/mach-tegra/tegra114/Makefile |2 +- > arch/arm/mach-tegra/tegra124/Makefile |2 - > arch/arm/mach-tegra/tegra20/Makefile |2 +- > arch/arm/mach-tegra/tegra210/Makefile |1 - > arch/arm/mach-tegra/tegra30/Makefile |2 +- > board/asus/grouper/grouper.c | 18 - > board/asus/grouper/pinmux-config-grouper.h| 362 -- > .../pinmux-config-transformer.h | 365 -- > board/asus/transformer-t30/transformer-t30.c | 23 - > board/compal/paz00/Makefile |8 - > board/compal/paz00/paz00.c| 53 - > board/htc/endeavoru/endeavoru.c | 14 - > board/htc/endeavoru/pinmux-config-endeavoru.h | 362 -- > board/lg/x3-t30/Kconfig | 12 - > board/lg/x3-t30/configs/p880.config |1 - > board/lg/x3-t30/configs/p895.config |1 - > board/lg/x3-t30/pinmux-config-x3.h| 449 --- > board/lg/x3-t30/x3-t30.c | 23 - > configs/paz00_defconfig |3 + > drivers/pinctrl/Kconfig |1 + > drivers/pinctrl/Makefile |1 + > drivers/pinctrl/tegra/Kconfig | 18 + > drivers/pinctrl/tegra/Makefile| 16 + > .../pinctrl/tegra/funcmux-tegra114.c |0 > .../pinctrl/tegra/funcmux-tegra124.c |0 > .../pinctrl/tegra/funcmux-tegra20.c |0 > .../pinctrl/tegra/funcmux-tegra210.c |0 > .../pinctrl/tegra/funcmux-tegra30.c |0 > drivers/pinctrl/tegra/pinctrl-tegra.c | 248 > drivers/pinctrl/tegra/pinctrl-tegra20.c | 177 +++ > .../pinctrl/tegra}/pinmux-common.c|0 > .../pinctrl/tegra/pinmux-tegra114.c |0 > .../pinctrl/tegra/pinmux-tegra124.c |0 > .../pinctrl/tegra/pinmux-tegra20.c|0 > drivers/pinctrl/tegra/pinmux-tegra210.c | 190 +++ > .../pinctrl/tegra/pinmux-tegra30.c|0 > include/configs/x3-t30.h | 13 +- > 63 files changed, 8920 insertions(+), 1723 deletions(-) So we're adding 7000 extra lines here just to get the same level of functionality? Do we really want this? For things like the kernel this makes a bit more sense because we can have generic kernel images for
Re: [PATCH v1 0/5] Convert recently merged T30 boards to use DM PMIC
On Mon, Nov 06, 2023 at 04:04:07PM -0500, Tom Rini wrote: > On Mon, Nov 06, 2023 at 02:11:16PM +, Peter Robinson wrote: > > On Mon, Nov 6, 2023 at 1:28 PM Svyatoslav Ryhel wrote: > > > > > > пн, 6 лист. 2023 р. о 15:13 Peter Robinson пише: > > > > > > > > On Mon, Nov 6, 2023 at 11:58 AM Svyatoslav Ryhel > > > > wrote: > > > > > > > > > > пн, 6 лист. 2023 р. о 13:46 Peter Robinson > > > > > пише: > > > > > > > > > > > > Hi Svyatoslav, > > > > > > > > > > > > > Since the proposed PMIC patches have been accepted, I see the need > > > > > > > to convert boards which I maintain to use DM drivers instead of > > > > > > > board hacks. > > > > > > > > > > > > > > Svyatoslav Ryhel (5): > > > > > > > board: lg-x3: convert LG Optimus 4X and Vu to use DM PMIC > > > > > > > board: endeavoru: convert HTC One X to use DM PMIC > > > > > > > > > > > > Is there a reason why the two above devices don't appear to have > > > > > > their > > > > > > .dts files in the upstream kernel? > > > > > > > > > > > > > > > > Yes, there is a reason. Linux maintainers treat submitters as > > > > > existential enemies or as dirt at least. I was trying to work with > > > > > linux but I have no desire to spend any time to upstream endeavoru or > > > > > lg_x3. > > > > > > > > The usual policy for acceptance into U-Boot is to have upstream review > > > > in the kernel first. > > > > > > > > > > May you point to a policy which clearly and explicitly states this as > > > a mandatory condition? > > > > There have been a number of devices rejected in the past until their > > DT are upstream but I'll leave Tom, who I've explicitly added on cc:, > > to clarify the exact policy. > > Well, here is where it's tricky. I brought this up for one of the > Broadcom MIPS platforms a week or two back, and Linus Walleij's point > (and I'm paraphrasing) is there's not really an upstream for it to go. > > What we cannot have is device tree bindings[1] that aren't upstream or > worse yet conflict with the official bindings. > > So the general way to resolve that is have device tree file be drop-in > from the linux kernel, and what additions we must have be done via > -u-boot.dtsi files. And in turn, some SoCs are better about keeping in > sync with the kernel than other SoCs are. > > Now, upstream being actively hostile to dts files, especially for older > platforms? That's unfortunate. So long as we aren't violating the rules > about bindings, the intention is that we don't have device trees that > are either (a) massively out of sync with the kernel[2] or (b) kept > intentionally mismatched from the kernel. > > -- > Tom > > [1]: There are both examples like binman that Simon is working on at > least but this is more exception than intentional rule. > [2]: Per our other conversions, I know the tegra ones are in this > unfortunate state in general On the Tegra side we've been fairly lax about the device trees in U-Boot, I suppose. The assumption had always been that U-Boot would load an external DTB and pass it to the kernel on boot, so keeping them both in sync was never a high priority. U-Boot does only a very tiny amount of what Linux does, so dropping in the kernel DTB always seemed a bit overkill. In either case, if this is problematic, it's something that I could take a look at. Again, it's expected that the device trees are different, for historical reasons, but I'd be surprised if they actually conflict with one another. U-Boot's DTB was always supposed to be a subset of the Linux DTB. Thierry signature.asc Description: PGP signature
Re: [PATCH v1 0/5] Convert recently merged T30 boards to use DM PMIC
On Mon, Nov 13, 2023 at 04:52:22PM +, Peter Robinson wrote: > > > > > > > > Since the proposed PMIC patches have been accepted, I see the > > > > > > > > need > > > > > > > > to convert boards which I maintain to use DM drivers instead of > > > > > > > > board hacks. > > > > > > > > > > > > > > > > Svyatoslav Ryhel (5): > > > > > > > > board: lg-x3: convert LG Optimus 4X and Vu to use DM PMIC > > > > > > > > board: endeavoru: convert HTC One X to use DM PMIC > > > > > > > > > > > > > > Is there a reason why the two above devices don't appear to have > > > > > > > their > > > > > > > .dts files in the upstream kernel? > > > > > > > > > > > > > > > > > > > Yes, there is a reason. Linux maintainers treat submitters as > > > > > > existential enemies or as dirt at least. I was trying to work with > > > > > > linux but I have no desire to spend any time to upstream endeavoru > > > > > > or > > > > > > lg_x3. > > > > > > > > > > The usual policy for acceptance into U-Boot is to have upstream review > > > > > in the kernel first. > > > > > > > > > > > > > May you point to a policy which clearly and explicitly states this as > > > > a mandatory condition? > > > > > > There have been a number of devices rejected in the past until their > > > DT are upstream but I'll leave Tom, who I've explicitly added on cc:, > > > to clarify the exact policy. > > > > Well, here is where it's tricky. I brought this up for one of the > > Broadcom MIPS platforms a week or two back, and Linus Walleij's point > > (and I'm paraphrasing) is there's not really an upstream for it to go. > > > > What we cannot have is device tree bindings[1] that aren't upstream or > > worse yet conflict with the official bindings. > > > > So the general way to resolve that is have device tree file be drop-in > > from the linux kernel, and what additions we must have be done via > > -u-boot.dtsi files. And in turn, some SoCs are better about keeping in > > sync with the kernel than other SoCs are. > > > > Now, upstream being actively hostile to dts files, especially for older > > platforms? That's unfortunate. So long as we aren't violating the rules > > about bindings, the intention is that we don't have device trees that > > are either (a) massively out of sync with the kernel[2] or (b) kept > > intentionally mismatched from the kernel. > > I don't believe I've seen upstream Tegra maintainers being actively > hostile towards updates for older devices, I know they have certainly > defocused them, but I'm not sure that's what I'd consider hostile. No objections from me on upstreaming older devices in Linux. I used to be able to test most of the older devices, but many of which I used to have direct access to are now defunct (for varying reasons). So I will have to rely on the community for testing etc. since I cannot scale to the point where I personally have all of these devices. Now, I don't think that's hostile and if I ever came across as hostile I'm sorry. The intent was never to reject device support. Obviously the Linux kernel has high standards and sometimes that can be off-putting, but I don't think we've ever asked for anything out of the ordinary. Thierry signature.asc Description: PGP signature
Re: [PATCH v2 0/2] tegra_mmc: get tap and trim from dts
From: Thierry Reding On Thu, 24 Aug 2023 21:53:30 +0300, Svyatoslav Ryhel wrote: > Default-tap and default-trim values are used for eMMC setup > mostly on T114+ devices. As for now, those values are hardcoded > for T210 and ignored for all other Tegra generations. Fix this > by passing tap and trim values from dts. > Applied, thanks! [1/2] ARM: tegra210: set default-tap and default-trim values in sdhci nodes (no commit info) [2/2] mmc: tegra: get default-tap and default-trim from device tree (no commit info) Best regards, -- Thierry Reding
Re: [PATCH v2 00/16] General tegra and board improvements
From: Thierry Reding On Thu, 24 Aug 2023 22:25:47 +0300, Svyatoslav Ryhel wrote: > This patchset follows Transformers, Grouper, LG X3 and Endeavoru > bringup and contains changes from v9 of previous patchset and > some new improvenets. > > List of changes: > - separated tf600t and p1801-t device trees since they use > different video path bindings > - enabled booting from usb devices (USB > SD > eMMC) > - fixed tf201 dock usb line binding > - removed transformer board pmic gpios setup > - refresh USB option converted into enter console for transformers > - updated device trees for future DM PMIC migration > - re-synced defconfigs > - added base voltages setup from board for t114 same as on t30 > - adjusted ebtupdate to work with non-encrypted re-crypted devices > - attempt to move setup of some env values to arch > - config fragments moved to board/vendor/device/configs/ dir > (requires pending u-boot patch) > > [...] Applied, thanks! [01/16] ARM: dts: p1801-t: separate from common transformers tree (no commit info) [02/16] ARM: dts: tf600t: separate from common transformers tree (no commit info) [03/16] configs: transformer_t30: support booting from USB (no commit info) [04/16] ARM: dts: tf201: configure dock USB phy (no commit info) [05/16] board: asus: transformer-t30: remove PMIC GPIOs configuration (no commit info) [06/16] configs: transformer_t30: convert bootmenu option (no commit info) [07/16] ARM: dts: transformer-t30: complete missing bindings (no commit info) [08/16] ARM: dts: endeavoru: complete missing bindings (no commit info) [09/16] ARM: dts: lg-x3: complete missing bindings (no commit info) [10/16] ARM: dts: grouper: complete missing bindings (no commit info) [11/16] configs: transformer_t30: grouper: lg-x3: endeavoru: sync defconfigs (no commit info) [12/16] ARM: tegra114: enable base voltages setup from board (no commit info) [13/16] ARM: tegra20: tegra30: support EBTUPDATE on non-encrypted devices (no commit info) [14/16] ARM: tegra: board2: add generic late init (no commit info) [15/16] board: tegra30: remove nvidia_board_late_init calls (no commit info) [16/16] board: asus: lg: move config fragments into device boards (no commit info) Best regards, -- Thierry Reding
Re: [PATCH v1 17/19] ARM: tegra: dt-setup: convert TrustZone remove into config
On Wed, Aug 23, 2023 at 02:47:11PM +0300, Svyatoslav Ryhel wrote: > > > 23 серпня 2023 р. 14:17:37 GMT+03:00, Thierry Reding > написав(-ла): > >On Tue, Aug 22, 2023 at 02:22:15PM +0300, Svyatoslav Ryhel wrote: > >> Remove of TrustZone nodes is required by many product devices > >> which require repetable calls of same function from device board. > >> To simplify this, TZ remove is converted into Kconfig option. > >> > >> Signed-off-by: Svyatoslav Ryhel > >> --- > >> arch/arm/mach-tegra/Kconfig| 14 ++ > >> arch/arm/mach-tegra/dt-setup.c | 7 +++ > >> 2 files changed, 21 insertions(+) > > > >I kind of preferred the original because it's very explicit. There's > >also no big advantage in consolidating this because the code is unlikely > >to ever require changing (the libfdt API is quite stable and these nodes > >are all hard-coded anyway). This new variant put non-generic code (it > >requires a Kconfig option after all) into a generic place, so it seems a > >bit backwards. > > So you say that I have to add same function to boards over and over > for all devices I plan to mainline? Unneeded code duplication in > action. There's a balance to be struck between duplication and readability. If you insert this board-specific code into a generic code path, that generic code path becomes difficult to read given the extra #ifdef guards etc. If you're really worried about duplicating code you can move the two libfdt calls into a separate function and call that function from each of the boards that need it. Thierry signature.asc Description: PGP signature
Re: [PATCH v1 1/1] mmc: tegra: get default-tap and default-trim from device tree
On Wed, Aug 23, 2023 at 03:02:42PM +0300, Svyatoslav Ryhel wrote:
>
>
> 23 серпня 2023 р. 14:03:25 GMT+03:00, Thierry Reding
> написав(-ла):
> >On Sat, Aug 19, 2023 at 06:35:01PM +0300, Svyatoslav Ryhel wrote:
> >> Default-tap and default-trim values are used for eMMC setup
> >> mostly on T114+ devices. As for now, those values are hardcoded
> >> for T210 and ignored for all other Tegra generations. Fix this
> >> by passing tap and trim values from dts.
> >>
> >> Tested-by: Svyatoslav Ryhel # ASUS TF701T
> >> Signed-off-by: Svyatoslav Ryhel
> >> ---
> >> arch/arm/include/asm/arch-tegra/tegra_mmc.h | 17
> >> drivers/mmc/tegra_mmc.c | 46 ++---
> >> 2 files changed, 30 insertions(+), 33 deletions(-)
> >>
> >> diff --git a/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> >> b/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> >> index d6a55764ba..750c7d809e 100644
> >> --- a/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> >> +++ b/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> >> @@ -128,21 +128,22 @@ struct tegra_mmc {
> >>
> >> /* SDMMC1/3 settings from SDMMCx Initialization Sequence of TRM */
> >> #define MEMCOMP_PADCTRL_VREF 7
> >> -#define AUTO_CAL_ENABLE (1 << 29)
> >> -#define AUTO_CAL_ACTIVE (1 << 31)
> >> -#define AUTO_CAL_START(1 << 31)
> >> +#define AUTO_CAL_ENABLE BIT(29)
> >> +#define AUTO_CAL_ACTIVE BIT(31)
> >> +#define AUTO_CAL_STARTBIT(31)
> >> +
> >> #if defined(CONFIG_TEGRA210)
> >> #define AUTO_CAL_PD_OFFSET(0x7D << 8)
> >> #define AUTO_CAL_PU_OFFSET(0 << 0)
> >> -#define IO_TRIM_BYPASS_MASK (1 << 2)
> >> -#define TRIM_VAL_SHIFT24
> >> -#define TRIM_VAL_MASK (0x1F << TRIM_VAL_SHIFT)
> >> -#define TAP_VAL_SHIFT 16
> >> -#define TAP_VAL_MASK (0xFF << TAP_VAL_SHIFT)
> >> #else
> >> #define AUTO_CAL_PD_OFFSET(0x70 << 8)
> >> #define AUTO_CAL_PU_OFFSET(0x62 << 0)
> >> #endif
> >>
> >> +#define TRIM_VAL_SHIFT24
> >> +#define TRIM_VAL_MASK (0x1F << TRIM_VAL_SHIFT)
> >> +#define TAP_VAL_SHIFT 16
> >> +#define TAP_VAL_MASK (0xFF << TAP_VAL_SHIFT)
> >> +
> >> #endif/* __ASSEMBLY__ */
> >> #endif/* __TEGRA_MMC_H_ */
> >> diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
> >> index f76fee3ea0..7627800261 100644
> >> --- a/drivers/mmc/tegra_mmc.c
> >> +++ b/drivers/mmc/tegra_mmc.c
> >> @@ -37,6 +37,9 @@ struct tegra_mmc_priv {
> >>unsigned int version; /* SDHCI spec. version */
> >>unsigned int clock; /* Current clock (MHz) */
> >>int mmc_id; /* peripheral id */
> >> +
> >> + u32 tap_value;
> >> + u32 trim_value;
> >> };
> >>
> >> static void tegra_mmc_set_power(struct tegra_mmc_priv *priv,
> >> @@ -526,31 +529,6 @@ static void tegra_mmc_pad_init(struct tegra_mmc_priv
> >> *priv)
> >>printf("%s: Warning: Autocal timed out!\n", __func__);
> >>/* TBD: Set CFG2TMC_SDMMC1_PAD_CAL_DRV* regs here */
> >>}
> >> -
> >> -#if defined(CONFIG_TEGRA210)
> >> - u32 tap_value, trim_value;
> >> -
> >> - /* Set tap/trim values for SDMMC1/3 @ <48MHz here */
> >> - val = readl(>reg->venspictl); /* aka VENDOR_SYS_SW_CNTL */
> >> - val &= IO_TRIM_BYPASS_MASK;
> >> - if (id == PERIPH_ID_SDMMC1) {
> >> - tap_value = 4; /* default */
> >> - if (val)
> >> - tap_value = 3;
> >> - trim_value = 2;
> >> - } else {/* SDMMC3 */
> >> - tap_value = 3;
> >> - trim_value = 3;
> >> - }
> >> -
> >> - val = readl(>reg->venclkctl);
> >> - val &= ~TRIM_VAL_MASK;
> >> - val |= (trim_value << TRIM_VAL_SHIFT);
> >> - val &= ~TAP_VAL_MASK;
> >> - val |= (tap_value << TAP_VAL_SHIFT);
> >> - writel(val, >reg->venclkctl);
> >> - debug("%s: VENDOR_CLOCK_CNTRL = 0x%08X\n", __func__, val);
> >> -#endif/* T210 */
> >> #endif/* T30/T210 */
> >> }
> >>
> >> @@ -588,6 +566,21 @@ static void tegra_mmc_reset(struct tegra_mmc_priv
> >> *priv, struct mmc *mmc)
> >>
> >>/* Make sure SDIO pads are set up */
> >>tegra_mmc_pad_init(priv);
> >> +
> >> + if (priv->tap_value || priv->trim_value) {
> >
> >I think 0 is a valid value for both tap and trim, so you want to be able
> >to write that. I suggest getting rid of the conditional and always
> >writing these values and rely on defaults to make sure that a good value
> >is always programmed.
> >
> >Alternatively if you really only want to program these when they've been
> >specified, use an extra variable (or something like -1 as a default
> >value) to discriminate.
>
> I may propose a compromise. Do not set default value at all and when time
> comes just check if tap or trim is not error.
Yes, that's essentially a variant of the second option and it should
work.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v1 1/1] mmc: tegra: get default-tap and default-trim from device tree
On Wed, Aug 23, 2023 at 02:38:48PM +0300, Svyatoslav Ryhel wrote:
>
>
> 23 серпня 2023 р. 14:03:25 GMT+03:00, Thierry Reding
> написав(-ла):
> >On Sat, Aug 19, 2023 at 06:35:01PM +0300, Svyatoslav Ryhel wrote:
> >> Default-tap and default-trim values are used for eMMC setup
> >> mostly on T114+ devices. As for now, those values are hardcoded
> >> for T210 and ignored for all other Tegra generations. Fix this
> >> by passing tap and trim values from dts.
> >>
> >> Tested-by: Svyatoslav Ryhel # ASUS TF701T
> >> Signed-off-by: Svyatoslav Ryhel
> >> ---
> >> arch/arm/include/asm/arch-tegra/tegra_mmc.h | 17
> >> drivers/mmc/tegra_mmc.c | 46 ++---
> >> 2 files changed, 30 insertions(+), 33 deletions(-)
> >>
> >> diff --git a/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> >> b/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> >> index d6a55764ba..750c7d809e 100644
> >> --- a/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> >> +++ b/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> >> @@ -128,21 +128,22 @@ struct tegra_mmc {
> >>
> >> /* SDMMC1/3 settings from SDMMCx Initialization Sequence of TRM */
> >> #define MEMCOMP_PADCTRL_VREF 7
> >> -#define AUTO_CAL_ENABLE (1 << 29)
> >> -#define AUTO_CAL_ACTIVE (1 << 31)
> >> -#define AUTO_CAL_START(1 << 31)
> >> +#define AUTO_CAL_ENABLE BIT(29)
> >> +#define AUTO_CAL_ACTIVE BIT(31)
> >> +#define AUTO_CAL_STARTBIT(31)
> >> +
> >> #if defined(CONFIG_TEGRA210)
> >> #define AUTO_CAL_PD_OFFSET(0x7D << 8)
> >> #define AUTO_CAL_PU_OFFSET(0 << 0)
> >> -#define IO_TRIM_BYPASS_MASK (1 << 2)
> >> -#define TRIM_VAL_SHIFT24
> >> -#define TRIM_VAL_MASK (0x1F << TRIM_VAL_SHIFT)
> >> -#define TAP_VAL_SHIFT 16
> >> -#define TAP_VAL_MASK (0xFF << TAP_VAL_SHIFT)
> >> #else
> >> #define AUTO_CAL_PD_OFFSET(0x70 << 8)
> >> #define AUTO_CAL_PU_OFFSET(0x62 << 0)
> >> #endif
> >>
> >> +#define TRIM_VAL_SHIFT24
> >> +#define TRIM_VAL_MASK (0x1F << TRIM_VAL_SHIFT)
> >> +#define TAP_VAL_SHIFT 16
> >> +#define TAP_VAL_MASK (0xFF << TAP_VAL_SHIFT)
> >> +
> >> #endif/* __ASSEMBLY__ */
> >> #endif/* __TEGRA_MMC_H_ */
> >> diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
> >> index f76fee3ea0..7627800261 100644
> >> --- a/drivers/mmc/tegra_mmc.c
> >> +++ b/drivers/mmc/tegra_mmc.c
> >> @@ -37,6 +37,9 @@ struct tegra_mmc_priv {
> >>unsigned int version; /* SDHCI spec. version */
> >>unsigned int clock; /* Current clock (MHz) */
> >>int mmc_id; /* peripheral id */
> >> +
> >> + u32 tap_value;
> >> + u32 trim_value;
> >> };
> >>
> >> static void tegra_mmc_set_power(struct tegra_mmc_priv *priv,
> >> @@ -526,31 +529,6 @@ static void tegra_mmc_pad_init(struct tegra_mmc_priv
> >> *priv)
> >>printf("%s: Warning: Autocal timed out!\n", __func__);
> >>/* TBD: Set CFG2TMC_SDMMC1_PAD_CAL_DRV* regs here */
> >>}
> >> -
> >> -#if defined(CONFIG_TEGRA210)
> >> - u32 tap_value, trim_value;
> >> -
> >> - /* Set tap/trim values for SDMMC1/3 @ <48MHz here */
> >> - val = readl(>reg->venspictl); /* aka VENDOR_SYS_SW_CNTL */
> >> - val &= IO_TRIM_BYPASS_MASK;
> >> - if (id == PERIPH_ID_SDMMC1) {
> >> - tap_value = 4; /* default */
> >> - if (val)
> >> - tap_value = 3;
> >> - trim_value = 2;
> >> - } else {/* SDMMC3 */
> >> - tap_value = 3;
> >> - trim_value = 3;
> >> - }
> >> -
> >> - val = readl(>reg->venclkctl);
> >> - val &= ~TRIM_VAL_MASK;
> >> - val |= (trim_value << TRIM_VAL_SHIFT);
> >> - val &= ~TAP_VAL_MASK;
> >> - val |= (tap_value << TAP_VAL_SHIFT);
> >> - writel(val, >reg->venclkctl);
> >> - debug("%s: VENDOR_CLOCK_CNTRL = 0x%08X\n", __func__, val);
> >> -#endif/* T210
Re: [PATCH v1 0/1] tegra_mmc: get tap and trim from dts
On Wed, Aug 23, 2023 at 02:30:48PM +0300, Svyatoslav Ryhel wrote: > > > 23 серпня 2023 р. 13:53:26 GMT+03:00, Thierry Reding > написав(-ла): > >On Sat, Aug 19, 2023 at 06:35:00PM +0300, Svyatoslav Ryhel wrote: > >> Default-tap and default-trim values are used for eMMC setup > >> mostly on T114+ devices. As for now, those values are hardcoded > >> for T210 and ignored for all other Tegra generations. Fix this > >> by passing tap and trim values from dts. > >> > >> This commit will cause regression on T210 (emmc may not work), > >> fix is applied in tegra210.dtsi and will be sent next week. > > > >Heh... I don't think so. If you already know that this will cause a > >regression on Tegra210 you need to rework this. Adding regressions > >accidentally is already bad enough, but doing so knowingly is a big > >no-no. > > DTS change for t210 was sent in "General tegra and board improvements" > patchset. This is why this pathset contains only 1 (one) patch and a > warning. It has a dependency. For U-Boot this may not matter as much because the control DTB is always linked into the binary, but for Linux for example we need to make sure that the kernel always remains backwards compatible with older device trees. I think that's good to follow in general, so making sure there are sensible defaults to fall back to if the DTB is missing some properties is good practice. Anyway, the way you posted these there was a future dependency, so if someone had applied the driver change without the DTB change having been applied first this would've caused a regression, which is always bad if it happens at random points because it makes things like bisections very painful. So unless the patch is fixed to fall back to defaults for Tegra210 we need to carefully coordinate in which order these patches go in. The DT change needs to go in first, followed by the driver change that relies on the DTB update. Thierry signature.asc Description: PGP signature
Re: [PATCH v1 17/19] ARM: tegra: dt-setup: convert TrustZone remove into config
On Tue, Aug 22, 2023 at 02:22:15PM +0300, Svyatoslav Ryhel wrote: > Remove of TrustZone nodes is required by many product devices > which require repetable calls of same function from device board. > To simplify this, TZ remove is converted into Kconfig option. > > Signed-off-by: Svyatoslav Ryhel > --- > arch/arm/mach-tegra/Kconfig| 14 ++ > arch/arm/mach-tegra/dt-setup.c | 7 +++ > 2 files changed, 21 insertions(+) I kind of preferred the original because it's very explicit. There's also no big advantage in consolidating this because the code is unlikely to ever require changing (the libfdt API is quite stable and these nodes are all hard-coded anyway). This new variant put non-generic code (it requires a Kconfig option after all) into a generic place, so it seems a bit backwards. Thierry signature.asc Description: PGP signature
Re: [PATCH v1 15/19] ARM: tegra: board2: add generic late init
On Tue, Aug 22, 2023 at 02:22:13PM +0300, Svyatoslav Ryhel wrote:
> Board specific late init allows vendors to set up different device
> or board specific env variables (like serial number, platform name).
> In case this information is missing, u-boot will lack info regards
> serial or platform.
>
> To avoid this prior nvidia_board_late_init internal generic function
> is called which fills required data. In this case platform name is
> obtained from get_chip and serialno is filled with SoC id.
>
> Though SoC id is not dedicated to be devices serial but it fits well
> in case of restriction of data about device and since SoC is basically
> a main chip of the device.
>
> Tested-by: Andreas Westman Dorcsak # ASUS Transformers
> Tested-by: Svyatoslav Ryhel # Nvidia Tegratab
> Signed-off-by: Svyatoslav Ryhel
> ---
> arch/arm/mach-tegra/board2.c | 43
> 1 file changed, 43 insertions(+)
>
> diff --git a/arch/arm/mach-tegra/board2.c b/arch/arm/mach-tegra/board2.c
> index 981768bb0e..ee69cb657a 100644
> --- a/arch/arm/mach-tegra/board2.c
> +++ b/arch/arm/mach-tegra/board2.c
> @@ -26,6 +26,10 @@
> #include
> #include
> #include
> +#ifndef CONFIG_TEGRA186
> +#include
> +#include
> +#endif
> #if IS_ENABLED(CONFIG_TEGRA_CLKRST)
> #include
> #endif
> @@ -256,6 +260,37 @@ int board_early_init_f(void)
> }
> #endif /* EARLY_INIT */
>
> +#ifndef CONFIG_TEGRA186
> +static void nvidia_board_late_init_generic(void)
> +{
> + char serialno_str[17];
> +
> + /* Set chip id as serialno */
> + sprintf(serialno_str, "%016llx", tegra_chip_uid());
> + env_set("serial#", serialno_str);
> +
> + switch (tegra_get_chip()) {
> + case CHIPID_TEGRA20:
> + env_set("platform", "Tegra 2 T20");
> + break;
> + case CHIPID_TEGRA30:
> + env_set("platform", "Tegra 3 T30");
> + break;
> + case CHIPID_TEGRA114:
> + env_set("platform", "Tegra 4 T114");
> + break;
> + case CHIPID_TEGRA124:
> + env_set("platform", "Tegra K1 T124");
> + break;
> + case CHIPID_TEGRA210:
> + env_set("platform", "Tegra X1 T210");
This variable is presumably something that you'd want to match on in a
script, so perhaps we can settle on something a bit more canonical. In
upstream Linux we use tegraXYZ quite consistently and you'll see that
reflected in things like compatible strings, so I'd suggest reusing the
same naming scheme here to simplify and avoid confusion.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v1 1/1] mmc: tegra: get default-tap and default-trim from device tree
On Sat, Aug 19, 2023 at 06:35:01PM +0300, Svyatoslav Ryhel wrote:
> Default-tap and default-trim values are used for eMMC setup
> mostly on T114+ devices. As for now, those values are hardcoded
> for T210 and ignored for all other Tegra generations. Fix this
> by passing tap and trim values from dts.
>
> Tested-by: Svyatoslav Ryhel # ASUS TF701T
> Signed-off-by: Svyatoslav Ryhel
> ---
> arch/arm/include/asm/arch-tegra/tegra_mmc.h | 17
> drivers/mmc/tegra_mmc.c | 46 ++---
> 2 files changed, 30 insertions(+), 33 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> b/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> index d6a55764ba..750c7d809e 100644
> --- a/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> +++ b/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> @@ -128,21 +128,22 @@ struct tegra_mmc {
>
> /* SDMMC1/3 settings from SDMMCx Initialization Sequence of TRM */
> #define MEMCOMP_PADCTRL_VREF 7
> -#define AUTO_CAL_ENABLE (1 << 29)
> -#define AUTO_CAL_ACTIVE (1 << 31)
> -#define AUTO_CAL_START (1 << 31)
> +#define AUTO_CAL_ENABLE BIT(29)
> +#define AUTO_CAL_ACTIVE BIT(31)
> +#define AUTO_CAL_START BIT(31)
> +
> #if defined(CONFIG_TEGRA210)
> #define AUTO_CAL_PD_OFFSET (0x7D << 8)
> #define AUTO_CAL_PU_OFFSET (0 << 0)
> -#define IO_TRIM_BYPASS_MASK (1 << 2)
> -#define TRIM_VAL_SHIFT 24
> -#define TRIM_VAL_MASK(0x1F << TRIM_VAL_SHIFT)
> -#define TAP_VAL_SHIFT16
> -#define TAP_VAL_MASK (0xFF << TAP_VAL_SHIFT)
> #else
> #define AUTO_CAL_PD_OFFSET (0x70 << 8)
> #define AUTO_CAL_PU_OFFSET (0x62 << 0)
> #endif
>
> +#define TRIM_VAL_SHIFT 24
> +#define TRIM_VAL_MASK(0x1F << TRIM_VAL_SHIFT)
> +#define TAP_VAL_SHIFT16
> +#define TAP_VAL_MASK (0xFF << TAP_VAL_SHIFT)
> +
> #endif /* __ASSEMBLY__ */
> #endif /* __TEGRA_MMC_H_ */
> diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
> index f76fee3ea0..7627800261 100644
> --- a/drivers/mmc/tegra_mmc.c
> +++ b/drivers/mmc/tegra_mmc.c
> @@ -37,6 +37,9 @@ struct tegra_mmc_priv {
> unsigned int version; /* SDHCI spec. version */
> unsigned int clock; /* Current clock (MHz) */
> int mmc_id; /* peripheral id */
> +
> + u32 tap_value;
> + u32 trim_value;
> };
>
> static void tegra_mmc_set_power(struct tegra_mmc_priv *priv,
> @@ -526,31 +529,6 @@ static void tegra_mmc_pad_init(struct tegra_mmc_priv
> *priv)
> printf("%s: Warning: Autocal timed out!\n", __func__);
> /* TBD: Set CFG2TMC_SDMMC1_PAD_CAL_DRV* regs here */
> }
> -
> -#if defined(CONFIG_TEGRA210)
> - u32 tap_value, trim_value;
> -
> - /* Set tap/trim values for SDMMC1/3 @ <48MHz here */
> - val = readl(>reg->venspictl); /* aka VENDOR_SYS_SW_CNTL */
> - val &= IO_TRIM_BYPASS_MASK;
> - if (id == PERIPH_ID_SDMMC1) {
> - tap_value = 4; /* default */
> - if (val)
> - tap_value = 3;
> - trim_value = 2;
> - } else {/* SDMMC3 */
> - tap_value = 3;
> - trim_value = 3;
> - }
> -
> - val = readl(>reg->venclkctl);
> - val &= ~TRIM_VAL_MASK;
> - val |= (trim_value << TRIM_VAL_SHIFT);
> - val &= ~TAP_VAL_MASK;
> - val |= (tap_value << TAP_VAL_SHIFT);
> - writel(val, >reg->venclkctl);
> - debug("%s: VENDOR_CLOCK_CNTRL = 0x%08X\n", __func__, val);
> -#endif /* T210 */
> #endif /* T30/T210 */
> }
>
> @@ -588,6 +566,21 @@ static void tegra_mmc_reset(struct tegra_mmc_priv *priv,
> struct mmc *mmc)
>
> /* Make sure SDIO pads are set up */
> tegra_mmc_pad_init(priv);
> +
> + if (priv->tap_value || priv->trim_value) {
I think 0 is a valid value for both tap and trim, so you want to be able
to write that. I suggest getting rid of the conditional and always
writing these values and rely on defaults to make sure that a good value
is always programmed.
Alternatively if you really only want to program these when they've been
specified, use an extra variable (or something like -1 as a default
value) to discriminate.
The former is superior, in my opinion, because it also allows to avoid
to regression.
Thierry
> + u32 val;
> +
> + val = readl(>reg->venclkctl);
> +
> + val &= ~TRIM_VAL_MASK;
> + val |= (priv->trim_value << TRIM_VAL_SHIFT);
> +
> + val &= ~TAP_VAL_MASK;
> + val |= (priv->tap_value << TAP_VAL_SHIFT);
> +
> + writel(val, >reg->venclkctl);
> + debug("%s: VENDOR_CLOCK_CNTRL = 0x%08X\n", __func__, val);
> + }
> }
>
> static int tegra_mmc_init(struct udevice *dev)
> @@ -742,6 +735,9 @@ static int tegra_mmc_probe(struct udevice
Re: [PATCH v1 0/1] tegra_mmc: get tap and trim from dts
On Sat, Aug 19, 2023 at 06:35:00PM +0300, Svyatoslav Ryhel wrote: > Default-tap and default-trim values are used for eMMC setup > mostly on T114+ devices. As for now, those values are hardcoded > for T210 and ignored for all other Tegra generations. Fix this > by passing tap and trim values from dts. > > This commit will cause regression on T210 (emmc may not work), > fix is applied in tegra210.dtsi and will be sent next week. Heh... I don't think so. If you already know that this will cause a regression on Tegra210 you need to rework this. Adding regressions accidentally is already bad enough, but doing so knowingly is a big no-no. Thierry signature.asc Description: PGP signature
Re: [PATCH v1 1/1] usb: host: tegra: implement dts based xcvr setup
On Sun, Aug 20, 2023 at 09:10:17PM +0200, Marek Vasut wrote:
> On 8/20/23 20:32, Svyatoslav Ryhel wrote:
> > 20 серпня 2023 р. 21:14:15 GMT+03:00, Marek Vasut
> > написав(-ла):
> > > On 8/20/23 09:13, Svyatoslav Ryhel wrote:
> > > > 20 серпня 2023 р. 05:23:14 GMT+03:00, Marek Vasut
> > > > написав(-ла):
> > > > > On 8/19/23 17:06, Svyatoslav Ryhel wrote:
> > > > > > Some devices (like ASUS TF201) may not have fuse based xcvr setup
> > > > > > which will result in not working USB line. To fix this situation
> > > > > > allow xcvr setup to be performed from device tree values if
> > > > > > nvidia,xcvr-setup-use-fuses is not defined.
> > > > > >
> > > > > > Tested-by: Svyatoslav Ryhel # ASUS TF201
> > > > >
> > > > > I would hope so. Usually T-B tags are not added by the patch author,
> > > > > but that's a detail, and here it has the added model value, so keep
> > > > > it.
> > > > >
> > > > > > Signed-off-by: Svyatoslav Ryhel
> > > > > > ---
> > > > > > drivers/usb/host/ehci-tegra.c | 53
> > > > > > +++
> > > > > > 1 file changed, 48 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/usb/host/ehci-tegra.c
> > > > > > b/drivers/usb/host/ehci-tegra.c
> > > > > > index 2cf1625670..f24baf8f0c 100644
> > > > > > --- a/drivers/usb/host/ehci-tegra.c
> > > > > > +++ b/drivers/usb/host/ehci-tegra.c
> > > > > > @@ -81,6 +81,8 @@ struct fdt_usb {
> > > > > >enum periph_id periph_id;/* peripheral id */
> > > > > >struct gpio_desc vbus_gpio; /* GPIO for vbus enable */
> > > > > >struct gpio_desc phy_reset_gpio; /* GPIO to reset ULPI phy */
> > > > > > +bool xcvr_setup_use_fuses; /* Indicates that the value is read
> > > > > > from the on-chip fuses */
> > > > > > +u32 xcvr_setup; /* Input of XCVR cell, HS driver output
> > > > > > control */
> > > > > > };
> > > > > > /*
> > > > > > @@ -464,10 +466,21 @@ static int init_utmi_usb_controller(struct
> > > > > > fdt_usb *config,
> > > > > >/* Recommended PHY settings for EYE diagram */
> > > > > >val = readl(>utmip_xcvr_cfg0);
> > > > > > -clrsetbits_le32(, UTMIP_XCVR_SETUP_MASK,
> > > > > > -0x4 << UTMIP_XCVR_SETUP_SHIFT);
> > > > > > -clrsetbits_le32(, UTMIP_XCVR_SETUP_MSB_MASK,
> > > > > > -0x3 << UTMIP_XCVR_SETUP_MSB_SHIFT);
> > > > > > +
> > > > > > +if (!config->xcvr_setup_use_fuses) {
> > > > > > +clrsetbits_le32(, UTMIP_XCVR_SETUP_MASK,
> > > > > > +config->xcvr_setup <<
> > > > > > +UTMIP_XCVR_SETUP_SHIFT);
> > > > > > +clrsetbits_le32(,
> > > > > > UTMIP_XCVR_SETUP_MSB_MASK,
> > > > > > +config->xcvr_setup <<
> > > > > > +UTMIP_XCVR_SETUP_MSB_SHIFT);
> > > > > > +} else {
> > > > > > +clrsetbits_le32(, UTMIP_XCVR_SETUP_MASK,
> > > > > > +0x4 << UTMIP_XCVR_SETUP_SHIFT);
> > > > >
> > > > > What is this hard-coded value ?
> > > > >
> > > >
> > > > 0x4 and 0x3 (not same) but those are not in the device tree. Mainline
> > > > linux
> > > > driver seems not set this at all if use_fuses is enabled but I decided
> > > > to keep
> > > > original setup just to not cause regressions.
> > > >
> > > > https://github.com/clamor-s/linux/blob/transformer/drivers/usb/phy/phy-tegra-usb.c#L587-L590
> > > >
> > > > > Why not place this value into config->xcvr_setup in e.g. probe() and
> > > > > drop this if/else statement ?
> > > > >
> > > >
> > > > Because config->xcvr_setup should matter only if use_fuses is disabled
> > >
> > > Can it matter always instead ?
> > >
> >
> > No, it cannot. You are inversing hw design. Xcvr_setup matters only if
> > use_fuses is disabled. If use_fuses is on (which is default state) xcvr
> > values are taken from chip fuse.
>
> The way I read this block of code is, some value is written into the
> register if config->xcvr_setup_use_fuses is false, another value if
> config->xcvr_setup_use_fuses is true . Why not do this determination once in
> probe and then just program the appropriate value instead ?
>
> > > > > > +clrsetbits_le32(,
> > > > > > UTMIP_XCVR_SETUP_MSB_MASK,
> > > > > > +0x3 <<
> > > > > > UTMIP_XCVR_SETUP_MSB_SHIFT);
> > > > > > +}
> > > > > > +
> > > > > >clrsetbits_le32(, UTMIP_XCVR_HSSLEW_MSB_MASK,
> > > > > >0x8 << UTMIP_XCVR_HSSLEW_MSB_SHIFT);
> > > > > >writel(val, >utmip_xcvr_cfg0);
> > > > > > @@ -522,7 +535,9 @@ static int init_utmi_usb_controller(struct
> > > > > > fdt_usb *config,
> > > > > >setbits_le32(>utmip_bat_chrg_cfg0, UTMIP_PD_CHRG);
> > > > > >
Re: [GIT PULL] ARM: tegra: Changes for v2023.10-rc1
On Tue, Aug 22, 2023 at 08:16:29AM +0300, Svyatoslav Ryhel wrote: > > > 18 серпня 2023 р. 20:04:47 GMT+03:00, Tom Rini > написав(-ла): > >On Fri, Aug 18, 2023 at 08:02:30PM +0300, Svyatoslav Ryhel wrote: > >> > >> > >> 18 серпня 2023 р. 19:49:43 GMT+03:00, Tom Rini > >> написав(-ла): > >> >On Fri, Aug 18, 2023 at 03:39:22PM +0200, Thierry Reding wrote: > >> > > >> >> From: Thierry Reding > >> >> > >> >> Hi Tom, > >> >> > >> >> The following changes since commit > >> >> 68c07fc5fdf34f0926cf06fc0c4ebd6f2f3afe19: > >> >> > >> >> Merge https://source.denx.de/u-boot/custodians/u-boot-usb (2023-06-21 > >> >> 14:42:50 -0400) > >> >> > >> >> are available in the Git repository at: > >> >> > >> >> g...@source.denx.de:u-boot/custodians/u-boot-tegra.git > >> >> tags/tegra-for-2023.10-rc1 > >> > > >> >FYI, in your .git/config you can do: > >> > url = https://source.denx.de/u-boot/custodians/u-boot-tegra.git > >> > pushurl = g...@source.denx.de:u-boot/custodians/u-boot-tegra.git > >> > > >> >And this part looks "nicer". Not a big deal, just an FYI. > >> > > >> >> for you to fetch changes up to bdf9dead86f06c7d6980c399a4a6339430b531ec: > >> >> > >> >> board: htc: endeavoru: add One X support (2023-06-30 15:20:37 +0200) > >> >> > >> >> This passes CI successfully: > >> >> > >> >> > >> >> https://source.denx.de/u-boot/custodians/u-boot-tegra/-/pipelines/17411 > >> >> > >> >> Thanks, > >> >> Thierry > >> >> > >> >> > >> >> ARM: tegra: Changes for v2023.10-rc1 > >> >> > >> >> This adds support for various new Tegra30 boards (ASUS, LG and HTC) and > >> >> has some other minor enhancements, such as enabling the poweroff command > >> >> on several Tegra210 and Tegra186 boards. > >> >> > >> >> > >> >> Jonas Schwöbel (1): > >> >> configs: tegra-common-post: make PXE and DHCP boot targets > >> >> optional > >> >> > >> >> Svyatoslav Ryhel (6): > >> >> configs: tegra-common-post: add GPIO keyboard as STDIN device > >> >> ARM: tegra: add SoC UID calculation function > >> >> board: asus: transformer: add ASUS Transformer T30 family support > >> >> board: asus: grouper: add Google Nexus 7 (2012) support > >> >> board: lg: x3: add Optimus 4X HD and Optimus Vu support > >> >> board: htc: endeavoru: add One X support > >> >> > >> >> Thierry Reding (2): > >> >> ARM: tegra: Enable poweroff command on Jetson TX1 and Jetson Nano > >> >> ARM: tegra: Enable poweroff command on Jetson TX2 > >> >> > >> >> Tomasz Maciej Nowak (1): > >> >> ARM: dts: trimslice: sync SPI node with Linux dts > >> > > >> >Since I'm not sure if this is v8 or v9, given when v9 was posted, I've > >> >applied this PR now, as I had said I wanted this in. I do have two > >> >requests for you Svyatoslav, however. First, if there's changes that > >> >are missing in master, please post those ASAP and we'll get them in, for > >> >v2023.10. > >> > >> Patches Thierry applied are v8, while v9 has significant improvements. > > > >OK, I've updated patchwork to try and reflect this too. > > > > Merged patches are not the most recent I have sent. How to fix this? Usually once something has been merged it's difficult to back it out. So unless Tom can (and is willing to) somehow redo the merge with the newest patches, you're going to have to rebase what you have and send out as follow-up patches. That's one of the reasons why you shouldn't keep sending new versions of patches once things have settled down and there haven't been any more review comments on the list. The assumption is that when you send patches out you think they are all done. They may still need to be updated based on review comments and that's fine because there's communication on the mailing list to indicate so. Once discussions have stopped it is assumed that the patches are done. This doesn't necessarily mean that they get picked up right away, because people might be busy. The prudent thing to do in such cases, in my experience, is to leave the patches as-is and do any subsequent changes in follow-up patches. If you quickly notice that there will need to be more changes, then let people know as soon as possible, otherwise they may apply patches and run tests on them, all of which will have to be redone if you repost a new version later on. In this particular case I was in the process of preparing the pull request to Tom and didn't see v9 until I had sent out the PR. Depending on who you're sending patches to the "point of no return" could be a lot sooner. Once patches are applied to a custodian tree and other patches have been applied on top, switching out new versions of patches can become really messy. Once they hit the mainline it often becomes impossible. Thierry signature.asc Description: PGP signature
Re: [GIT PULL] ARM: tegra: Changes for v2023.10-rc1
On Fri, Aug 18, 2023 at 08:02:30PM +0300, Svyatoslav Ryhel wrote: > > > 18 серпня 2023 р. 19:49:43 GMT+03:00, Tom Rini > написав(-ла): > >On Fri, Aug 18, 2023 at 03:39:22PM +0200, Thierry Reding wrote: > > > >> From: Thierry Reding > >> > >> Hi Tom, > >> > >> The following changes since commit > >> 68c07fc5fdf34f0926cf06fc0c4ebd6f2f3afe19: > >> > >> Merge https://source.denx.de/u-boot/custodians/u-boot-usb (2023-06-21 > >> 14:42:50 -0400) > >> > >> are available in the Git repository at: > >> > >> g...@source.denx.de:u-boot/custodians/u-boot-tegra.git > >> tags/tegra-for-2023.10-rc1 > > > >FYI, in your .git/config you can do: > > url = https://source.denx.de/u-boot/custodians/u-boot-tegra.git > > pushurl = g...@source.denx.de:u-boot/custodians/u-boot-tegra.git > > > >And this part looks "nicer". Not a big deal, just an FYI. > > > >> for you to fetch changes up to bdf9dead86f06c7d6980c399a4a6339430b531ec: > >> > >> board: htc: endeavoru: add One X support (2023-06-30 15:20:37 +0200) > >> > >> This passes CI successfully: > >> > >> https://source.denx.de/u-boot/custodians/u-boot-tegra/-/pipelines/17411 > >> > >> Thanks, > >> Thierry > >> > >> > >> ARM: tegra: Changes for v2023.10-rc1 > >> > >> This adds support for various new Tegra30 boards (ASUS, LG and HTC) and > >> has some other minor enhancements, such as enabling the poweroff command > >> on several Tegra210 and Tegra186 boards. > >> > >> > >> Jonas Schwöbel (1): > >> configs: tegra-common-post: make PXE and DHCP boot targets optional > >> > >> Svyatoslav Ryhel (6): > >> configs: tegra-common-post: add GPIO keyboard as STDIN device > >> ARM: tegra: add SoC UID calculation function > >> board: asus: transformer: add ASUS Transformer T30 family support > >> board: asus: grouper: add Google Nexus 7 (2012) support > >> board: lg: x3: add Optimus 4X HD and Optimus Vu support > >> board: htc: endeavoru: add One X support > >> > >> Thierry Reding (2): > >> ARM: tegra: Enable poweroff command on Jetson TX1 and Jetson Nano > >> ARM: tegra: Enable poweroff command on Jetson TX2 > >> > >> Tomasz Maciej Nowak (1): > >> ARM: dts: trimslice: sync SPI node with Linux dts > > > >Since I'm not sure if this is v8 or v9, given when v9 was posted, I've > >applied this PR now, as I had said I wanted this in. I do have two > >requests for you Svyatoslav, however. First, if there's changes that > >are missing in master, please post those ASAP and we'll get them in, for > >v2023.10. > > Patches Thierry applied are v8, while v9 has significant improvements. I'm a little surprised by how this series developed. You've been urging me since v6 to apply this, so I assumed this was mostly done. Having another 3 versions with significant improvements come in after that point indicates the opposite was true. Thierry signature.asc Description: PGP signature
Re: [GIT PULL] ARM: tegra: Changes for v2023.10-rc1
On Fri, Aug 18, 2023 at 12:49:43PM -0400, Tom Rini wrote: > On Fri, Aug 18, 2023 at 03:39:22PM +0200, Thierry Reding wrote: > > > From: Thierry Reding > > > > Hi Tom, > > > > The following changes since commit 68c07fc5fdf34f0926cf06fc0c4ebd6f2f3afe19: > > > > Merge https://source.denx.de/u-boot/custodians/u-boot-usb (2023-06-21 > > 14:42:50 -0400) > > > > are available in the Git repository at: > > > > g...@source.denx.de:u-boot/custodians/u-boot-tegra.git > > tags/tegra-for-2023.10-rc1 > > FYI, in your .git/config you can do: > url = https://source.denx.de/u-boot/custodians/u-boot-tegra.git > pushurl = g...@source.denx.de:u-boot/custodians/u-boot-tegra.git > > And this part looks "nicer". Not a big deal, just an FYI. This actually comes from a script that doesn't use the git config but takes this from gitlab directly using the REST API. This is primarily because the script also gets the link to the CI pipeline from there so it was convenient. Anyway, I wasn't sure if you preferred the HTTPS link or the SSH link, but I can update the script now that I know. =) Thierry signature.asc Description: PGP signature
[GIT PULL] ARM: tegra: Changes for v2023.10-rc1
From: Thierry Reding Hi Tom, The following changes since commit 68c07fc5fdf34f0926cf06fc0c4ebd6f2f3afe19: Merge https://source.denx.de/u-boot/custodians/u-boot-usb (2023-06-21 14:42:50 -0400) are available in the Git repository at: g...@source.denx.de:u-boot/custodians/u-boot-tegra.git tags/tegra-for-2023.10-rc1 for you to fetch changes up to bdf9dead86f06c7d6980c399a4a6339430b531ec: board: htc: endeavoru: add One X support (2023-06-30 15:20:37 +0200) This passes CI successfully: https://source.denx.de/u-boot/custodians/u-boot-tegra/-/pipelines/17411 Thanks, Thierry ARM: tegra: Changes for v2023.10-rc1 This adds support for various new Tegra30 boards (ASUS, LG and HTC) and has some other minor enhancements, such as enabling the poweroff command on several Tegra210 and Tegra186 boards. Jonas Schwöbel (1): configs: tegra-common-post: make PXE and DHCP boot targets optional Svyatoslav Ryhel (6): configs: tegra-common-post: add GPIO keyboard as STDIN device ARM: tegra: add SoC UID calculation function board: asus: transformer: add ASUS Transformer T30 family support board: asus: grouper: add Google Nexus 7 (2012) support board: lg: x3: add Optimus 4X HD and Optimus Vu support board: htc: endeavoru: add One X support Thierry Reding (2): ARM: tegra: Enable poweroff command on Jetson TX1 and Jetson Nano ARM: tegra: Enable poweroff command on Jetson TX2 Tomasz Maciej Nowak (1): ARM: dts: trimslice: sync SPI node with Linux dts arch/arm/dts/Makefile | 13 + arch/arm/dts/tegra20-trimslice.dts | 8 +- arch/arm/dts/tegra30-asus-grouper-common.dtsi | 157 +++ arch/arm/dts/tegra30-asus-nexus7-grouper-E1565.dts | 43 ++ arch/arm/dts/tegra30-asus-nexus7-grouper-PM269.dts | 36 ++ arch/arm/dts/tegra30-asus-nexus7-tilapia-E1565.dts | 62 +++ arch/arm/dts/tegra30-asus-p1801-t.dts | 18 + arch/arm/dts/tegra30-asus-tf201.dts| 9 + arch/arm/dts/tegra30-asus-tf300t.dts | 18 + arch/arm/dts/tegra30-asus-tf300tg.dts | 9 + arch/arm/dts/tegra30-asus-tf300tl.dts | 9 + arch/arm/dts/tegra30-asus-tf600t.dts | 89 arch/arm/dts/tegra30-asus-tf700t.dts | 13 + arch/arm/dts/tegra30-asus-transformer.dtsi | 211 ++ arch/arm/dts/tegra30-htc-endeavoru.dts | 166 arch/arm/dts/tegra30-lg-p880.dts | 40 ++ arch/arm/dts/tegra30-lg-p895.dts | 50 +++ arch/arm/dts/tegra30-lg-x3.dtsi| 180 + arch/arm/include/asm/arch-tegra/fuse.h | 7 + arch/arm/mach-tegra/Makefile | 4 + arch/arm/mach-tegra/fuse.c | 151 +++ arch/arm/mach-tegra/tegra30/Kconfig| 20 + board/asus/grouper/Kconfig | 22 + board/asus/grouper/MAINTAINERS | 10 + board/asus/grouper/Makefile| 14 + board/asus/grouper/grouper-spl-max.c | 45 +++ board/asus/grouper/grouper-spl-ti.c| 41 ++ board/asus/grouper/grouper.c | 202 + board/asus/grouper/pinmux-config-grouper.h | 362 + board/asus/transformer-t30/Kconfig | 23 ++ board/asus/transformer-t30/MAINTAINERS | 15 + board/asus/transformer-t30/Makefile| 11 + .../transformer-t30/pinmux-config-transformer.h| 365 + board/asus/transformer-t30/transformer-t30-spl.c | 41 ++ board/asus/transformer-t30/transformer-t30.c | 201 + board/htc/endeavoru/Kconfig| 12 + board/htc/endeavoru/MAINTAINERS| 7 + board/htc/endeavoru/Makefile | 11 + board/htc/endeavoru/endeavoru-spl.c| 47 +++ board/htc/endeavoru/endeavoru.c| 116 ++ board/htc/endeavoru/pinmux-config-endeavoru.h | 362 + board/lg/x3-t30/Kconfig| 26 ++ board/lg/x3-t30/MAINTAINERS| 9 + board/lg/x3-t30/Makefile | 11 + board/lg/x3-t30/pinmux-config-x3.h | 449 + board/lg/x3-t30/x3-t30-spl.c | 48 +++ board/lg/x3-t30/x3-t30.c | 176 configs/endeavoru_defconfig| 84 configs/grouper_E1565.config | 2 + configs/grouper_PM269.config | 2 + configs/grouper_common_defconfig | 84 configs/p1801-t.config | 2 + configs/p2371-2180_defconfig | 1
Re: [PATCH v8 0/7] Tegra: add ASUS/Google Nexus 7 (2012) support
On Mon, Aug 07, 2023 at 09:49:07AM -0400, Tom Rini wrote: > On Mon, Jul 31, 2023 at 11:29:19AM -0400, Tom Rini wrote: > > On Mon, Jul 31, 2023 at 03:19:08PM +0300, Svyatoslav Ryhel wrote: > > > Hello! > > > > > > It has been a month since the last patchset was sent. Should I re-send > > > them? > > > > Not unless things don't apply. Thierry, can you put together a pull > > request soon? Thanks! > > Thierry? I'm OK with taking these boards after -rc2 if that helps move > things along, thanks. Sorry, I've been out of office for the last couple of weeks. Let me put this high up on my list of things to get back to once I'm all caught up with email. Thierry signature.asc Description: PGP signature
Re: [PATCH v7 4/7] board: asus: transformer: add ASUS Transformer T30 family support
On Fri, Jun 23, 2023 at 05:19:09PM +0300, Svyatoslav Ryhel wrote: > > > 23 червня 2023 р. 17:11:35 GMT+03:00, Thierry Reding > написав(-ла): > >On Fri, Jun 23, 2023 at 02:51:54PM +0300, Svyatoslav Ryhel wrote: > >> > >> > >> 23 червня 2023 р. 14:32:30 GMT+03:00, Thierry Reding > >> написав(-ла): > >> >On Fri, Jun 23, 2023 at 08:55:57AM +0300, Svyatoslav Ryhel wrote: > >> >[...] > >> >> diff --git a/board/asus/transformer-t30/pinmux-config-transformer.h > >> >> b/board/asus/transformer-t30/pinmux-config-transformer.h > >> >> new file mode 100644 > >> >> index 00..96ff45d375 > >> >> --- /dev/null > >> >> +++ b/board/asus/transformer-t30/pinmux-config-transformer.h > >> >> @@ -0,0 +1,365 @@ > >> >> +/* SPDX-License-Identifier: GPL-2.0 */ > >> >> +/* > >> >> + * Copyright (c) 2010-2013, NVIDIA CORPORATION. All rights reserved. > >> >> + * > >> >> + * Copyright (c) 2021, Svyatoslav Ryhel. > >> >> + */ > >> > > >> >I don't recall if we discussed this during upstreaming of the Linux > >> >device tree files, but shortly after the initial support for Tegra20 and > >> >Tegra30 was upstreamed (both in U-Boot and in Linux) we decided to move > >> >towards generating the pinmux configuration for the various consumers > >> >(i.e. U-Boot and Linux) from data tables using a set of scripts. This > >> >was done because we noticed that various inconsistencies kept creeping > >> >into the various drivers/tables. > >> > > >> >You can find the scripts for this here: > >> > > >> > https://github.com/NVIDIA/tegra-pinmux-scripts > >> > > >> >I think there'd be some benefit if these new boards were also converted > >> >to use these scripts. > >> > >> True, but there is no driver for pinmux which can utilize device tree, > >> instead u-boot relies on board header. I see the benefit of these pinmux > >> nodes only if I can verify that they are correct and do not brake devices. > >> Would be really bad if at some point devices brake cause nodes were not > >> tested. > >> > >> Additionally, shouldn't all supported boards be converted first? > > > >The above repository contains a script that will generate the board > >headers for U-Boot based on a more generic definition of the pinmux > >configuration. Many devices already have headers that were generated > >from those scripts, though it's mostly later boards (Tegra124 and > >onwards). > > > >That said, for many of these boards a spreadsheet exists that contains > >these settings and the tables are generated from that spreadsheet. For > >many of these OEM devices I suspect no such spreadsheet exists, and it > >doesn't necessarily make sense to transcribe this into the tables and > >then convert into board header & DTS snippets. It might be nice to do > >for consistency, but I'll leave it up to you. > > Current tables are dumped from working vendor kernels. It is not that > I like this approach, but I would rather leave an existing setup since > it is well tested and confirmed to be stable enough. No one knows what > htc and asus did with those pins, at least publicly. Okay, fair enough. Thierry signature.asc Description: PGP signature
Re: [PATCH v7 3/7] ARM: tegra: add SoC UID calculation function
On Fri, Jun 23, 2023 at 02:46:54PM +0300, Svyatoslav Ryhel wrote: > > > 23 червня 2023 р. 14:24:37 GMT+03:00, Thierry Reding > написав(-ла): > >On Fri, Jun 23, 2023 at 08:55:56AM +0300, Svyatoslav Ryhel wrote: > >> This is a small tool for calculation of SoC UID based on the same > >> Linux function. It can be further used for generation of device > >> unique data like mac address or exposing it as serial number. > > > >It's a very bad idea to use the SoC UID as a MAC address. There are > >better ways (such as MAC address randomization) to generate one if for > >some reason you don't have a real MAC address or are concerned about > >privacy. > > SoC UID is not used directly as MAC but it is used as a device > specific base to generate device specific one. You can check LG board > to see what I mean. Is this something that originates from the original vendor code? My primary concern is that this might end up reusing MAC addresses which were assigned to other devices. > > >The SoC UID is also not very well suited as a serial number because it > >identifies only the SoC, but doesn't say anything about any of the other > >components of a device. Many devices have serial numbers in some EEPROM > >chip, so those would be more appropriate. > > That is not the case of devices in patches and IIRC SoC UID is used as > fastboot ID on transformers by vendor. That doesn't really make this a better idea, but I also understand that your options are limited given the information you have. > >I suppose not all devices have such a system-wide serial number, so > >perhaps there are cases where this would be better than nothing. > > Vendors usually do not expose serial in any device hardware, or at > least do not bother to inform, where to find it. My experience differs. There's usually some serial number somewhere because vendors need some way of tracking these devices. But yeah, if you get an OEM device they typically don't tell you where to find it. One thing you might want to do is probe the various I2C busses to see if there's an EEPROM on any of them. They are often found at addresses 0x50-0x58 or so. Again, I'm not strongly objecting to this, but I'd prefer some better way to identify system than by chip UID, because it's not meant for this purpose. Thierry signature.asc Description: PGP signature
Re: [PATCH v7 4/7] board: asus: transformer: add ASUS Transformer T30 family support
On Fri, Jun 23, 2023 at 02:51:54PM +0300, Svyatoslav Ryhel wrote: > > > 23 червня 2023 р. 14:32:30 GMT+03:00, Thierry Reding > написав(-ла): > >On Fri, Jun 23, 2023 at 08:55:57AM +0300, Svyatoslav Ryhel wrote: > >[...] > >> diff --git a/board/asus/transformer-t30/pinmux-config-transformer.h > >> b/board/asus/transformer-t30/pinmux-config-transformer.h > >> new file mode 100644 > >> index 00..96ff45d375 > >> --- /dev/null > >> +++ b/board/asus/transformer-t30/pinmux-config-transformer.h > >> @@ -0,0 +1,365 @@ > >> +/* SPDX-License-Identifier: GPL-2.0 */ > >> +/* > >> + * Copyright (c) 2010-2013, NVIDIA CORPORATION. All rights reserved. > >> + * > >> + * Copyright (c) 2021, Svyatoslav Ryhel. > >> + */ > > > >I don't recall if we discussed this during upstreaming of the Linux > >device tree files, but shortly after the initial support for Tegra20 and > >Tegra30 was upstreamed (both in U-Boot and in Linux) we decided to move > >towards generating the pinmux configuration for the various consumers > >(i.e. U-Boot and Linux) from data tables using a set of scripts. This > >was done because we noticed that various inconsistencies kept creeping > >into the various drivers/tables. > > > >You can find the scripts for this here: > > > > https://github.com/NVIDIA/tegra-pinmux-scripts > > > >I think there'd be some benefit if these new boards were also converted > >to use these scripts. > > True, but there is no driver for pinmux which can utilize device tree, > instead u-boot relies on board header. I see the benefit of these pinmux > nodes only if I can verify that they are correct and do not brake devices. > Would be really bad if at some point devices brake cause nodes were not > tested. > > Additionally, shouldn't all supported boards be converted first? The above repository contains a script that will generate the board headers for U-Boot based on a more generic definition of the pinmux configuration. Many devices already have headers that were generated from those scripts, though it's mostly later boards (Tegra124 and onwards). That said, for many of these boards a spreadsheet exists that contains these settings and the tables are generated from that spreadsheet. For many of these OEM devices I suspect no such spreadsheet exists, and it doesn't necessarily make sense to transcribe this into the tables and then convert into board header & DTS snippets. It might be nice to do for consistency, but I'll leave it up to you. The repository has been in an archived state since there wasn't any activity on it for a long time and I guess we didn't expect any new boards to be added at this point. If needed we could revive this. Thierry signature.asc Description: PGP signature
Re: [PATCH v7 4/7] board: asus: transformer: add ASUS Transformer T30 family support
On Fri, Jun 23, 2023 at 08:55:57AM +0300, Svyatoslav Ryhel wrote: [...] > diff --git a/board/asus/transformer-t30/pinmux-config-transformer.h > b/board/asus/transformer-t30/pinmux-config-transformer.h > new file mode 100644 > index 00..96ff45d375 > --- /dev/null > +++ b/board/asus/transformer-t30/pinmux-config-transformer.h > @@ -0,0 +1,365 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2010-2013, NVIDIA CORPORATION. All rights reserved. > + * > + * Copyright (c) 2021, Svyatoslav Ryhel. > + */ I don't recall if we discussed this during upstreaming of the Linux device tree files, but shortly after the initial support for Tegra20 and Tegra30 was upstreamed (both in U-Boot and in Linux) we decided to move towards generating the pinmux configuration for the various consumers (i.e. U-Boot and Linux) from data tables using a set of scripts. This was done because we noticed that various inconsistencies kept creeping into the various drivers/tables. You can find the scripts for this here: https://github.com/NVIDIA/tegra-pinmux-scripts I think there'd be some benefit if these new boards were also converted to use these scripts. Thierry signature.asc Description: PGP signature
Re: [PATCH v7 3/7] ARM: tegra: add SoC UID calculation function
On Fri, Jun 23, 2023 at 08:55:56AM +0300, Svyatoslav Ryhel wrote: > This is a small tool for calculation of SoC UID based on the same > Linux function. It can be further used for generation of device > unique data like mac address or exposing it as serial number. It's a very bad idea to use the SoC UID as a MAC address. There are better ways (such as MAC address randomization) to generate one if for some reason you don't have a real MAC address or are concerned about privacy. The SoC UID is also not very well suited as a serial number because it identifies only the SoC, but doesn't say anything about any of the other components of a device. Many devices have serial numbers in some EEPROM chip, so those would be more appropriate. I suppose not all devices have such a system-wide serial number, so perhaps there are cases where this would be better than nothing. Thierry signature.asc Description: PGP signature
Re: [PATCH v3 00/16] General Tegra improvements
board/toradex/apalis_t30/apalis_t30-spl.c | 34 +++
> board/toradex/colibri_t30/Makefile| 2 +
> board/toradex/colibri_t30/colibri_t30-spl.c | 34 +++
> configs/beaver_defconfig | 3 -
> configs/cei-tk1-som_defconfig | 3 -
> configs/dalmore_defconfig | 3 -
> configs/jetson-tk1_defconfig | 3 -
> configs/nyan-big_defconfig| 3 -
> configs/p2371-_defconfig | 3 -
> configs/p2371-2180_defconfig | 3 -
> configs/p2571_defconfig | 3 -
> configs/p3450-_defconfig | 3 -
> configs/venice2_defconfig | 3 -
> doc/usage/cmd/ebtupdate.rst | 70 +++
> drivers/pwm/tegra_pwm.c | 10 ++-
> drivers/spi/tegra20_slink.c | 19 ++--
> drivers/usb/gadget/Kconfig| 3 +
> 57 files changed, 1051 insertions(+), 309 deletions(-)
> create mode 100644 arch/arm/include/asm/arch-tegra/crypto.h
> rename arch/arm/mach-tegra/{tegra20 => }/crypto.c (68%)
> create mode 100644 arch/arm/mach-tegra/tegra20/bct.c
> create mode 100644 arch/arm/mach-tegra/tegra20/bct.h
> delete mode 100644 arch/arm/mach-tegra/tegra20/crypto.h
> create mode 100644 arch/arm/mach-tegra/tegra30/bct.c
> create mode 100644 arch/arm/mach-tegra/tegra30/bct.h
> create mode 100644 board/avionic-design/tec-ng/tec-ng-spl.c
> create mode 100644 board/nvidia/beaver/beaver-spl.c
> create mode 100644 board/nvidia/cardhu/cardhu-spl.c
> delete mode 100644 board/nvidia/venice2/as3722_init.h
> delete mode 100644 board/toradex/apalis-tk1/as3722_init.h
> create mode 100644 board/toradex/apalis_t30/apalis_t30-spl.c
> create mode 100644 board/toradex/colibri_t30/colibri_t30-spl.c
> create mode 100644 doc/usage/cmd/ebtupdate.rst
I haven't tested this exhaustively, just merely made sure that it can
PXE boot into Linux. Seems to work fine on Beaver and Jetson TK1, so:
Tested-by: Thierry Reding
signature.asc
Description: PGP signature
Re: [PATCH v8 0/3] Timer support for ARM Tegra
On Wed, Feb 01, 2023 at 10:53:00AM +0200, Svyatoslav Ryhel wrote: > - ARM: tegra: remap clock_osc_freq for all Tegra family > Enum clock_osc_freq was designed to use only with T20. > This patch remaps it to use additional frequencies, added in > T30+ SoC while maintaining backwards compatibility with T20. > > - drivers: timer: add timer driver for ARMv7 based Tegra devices > Add timer support for T20/T30/T114/T124 and T210 based devices. > Driver is based on DM, has device tree support and can be > used on SPL and early boot stage. > > Arm64 Tegra (apart T210) according to comment in tegra-common.h use > architected timer. > > - ARM: tegra: include timer as default option > Enable TIMER and TEGRA_TIMER for TEGRA_ARMV7_COMMON and TEGRA210. > Additionally enable SPL_TIMER if build as SPL part and drop > deprecated configs from common header. > > --- > Changed from v7 > - configured timer selection only for armv7 Tegra and T210 > > Changed from v6 > - use clk_m as timer calibration clock (this should properly fix T210) > - enable timer for T210 > > Changed from v5: > - added paz00 tester > > Changed from v4: > - added comments > > Changed from v3: > - removed BOOTSTAGE ifdefs > - use early timer on boot stage unconditionally > --- > > Svyatoslav Ryhel (3): > ARM: tegra: remap clock_osc_freq for all Tegra family > drivers: timer: add driver for ARMv7 based Tegra devices and T210 > ARM: tegra: include timer as default option > > arch/arm/Kconfig| 1 + > arch/arm/include/asm/arch-tegra/clock.h | 9 +- > arch/arm/mach-tegra/Kconfig | 4 + > arch/arm/mach-tegra/clock.c | 17 +++- > arch/arm/mach-tegra/cpu.c | 70 ++--- > arch/arm/mach-tegra/tegra114/clock.c| 13 +-- > arch/arm/mach-tegra/tegra124/clock.c| 13 +-- > arch/arm/mach-tegra/tegra20/clock.c | 4 +- > arch/arm/mach-tegra/tegra210/clock.c| 22 +--- > arch/arm/mach-tegra/tegra30/clock.c | 10 +- > drivers/timer/Kconfig | 8 ++ > drivers/timer/Makefile | 1 + > drivers/timer/tegra-timer.c | 130 > drivers/usb/host/ehci-tegra.c | 46 +++-- > include/configs/tegra-common.h | 6 -- > 15 files changed, 275 insertions(+), 79 deletions(-) > create mode 100644 drivers/timer/tegra-timer.c This works fine on Beaver (Tegra30), Jetson TK1 (Tegra124), Jetson TX1 (Tegra210) and Jetson TX2 (Tegra186), so: Tested-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH v7 0/3] Timer support for ARM Tegra
On Fri, Jan 27, 2023 at 10:27:57PM +0200, Svyatoslav R. wrote: > On 1/27/23 19:15, Thierry Reding wrote: > > On Fri, Jan 27, 2023 at 09:13:09AM +0200, Svyatoslav Ryhel wrote: > > > - ARM: tegra: remap clock_osc_freq for all Tegra family > > > Enum clock_osc_freq was designed to use only with T20. > > > This patch remaps it to use additional frequencies, added in > > > T30+ SoC while maintaining backwards compatibility with T20. > > > > > > - drivers: timer: add timer driver for ARMv7 based Tegra devices > > > Add timer support for T20/T30/T114/T124 and T210 based devices. > > > Driver is based on DM, has device tree support and can be > > > used on SPL and early boot stage. > > > > > > Arm64 Tegra (apart T210) according to comment in tegra-common.h use > > > architected timer. > > > > > > - ARM: tegra: include timer as default option > > > Enable TIMER as default option for all Tegra devices and > > > enable TEGRA_TIMER for TEGRA_ARMV7_COMMON and TEGRA210. > > > Additionally enable SPL_TIMER if build as SPL part and > > > drop deprecated configs from common header. > > > > > > P. S. I have no arm64 Tegra and according to comment in > > > tegra-common.h > > > Use the Tegra US timer on ARMv7, but the architected timer on ARMv8. > > > > > > --- > > > Changeog from V6 > > > - use clk_m as timer calibration clock (this should properly fix T210) > > > - enable timer for T210 > > > > > > Changed from v5: > > > - added paz00 tester > > > > > > Changed from v4: > > > - added comments > > > > > > Changed from v3: > > > - removed BOOTSTAGE ifdefs > > > - use early timer on boot stage unconditionally > > > --- > > > Svyatoslav Ryhel (3): > > >ARM: tegra: remap clock_osc_freq for all Tegra family > > >drivers: timer: add driver for ARMv7 based Tegra devices and T210 > > >ARM: tegra: include timer as default option > > > > > > arch/arm/Kconfig| 1 + > > > arch/arm/include/asm/arch-tegra/clock.h | 9 +- > > > arch/arm/mach-tegra/Kconfig | 3 + > > > arch/arm/mach-tegra/clock.c | 17 +++- > > > arch/arm/mach-tegra/cpu.c | 70 ++--- > > > arch/arm/mach-tegra/tegra114/clock.c| 13 +-- > > > arch/arm/mach-tegra/tegra124/clock.c| 13 +-- > > > arch/arm/mach-tegra/tegra20/clock.c | 4 +- > > > arch/arm/mach-tegra/tegra210/clock.c| 22 +--- > > > arch/arm/mach-tegra/tegra30/clock.c | 10 +- > > > drivers/timer/Kconfig | 8 ++ > > > drivers/timer/Makefile | 1 + > > > drivers/timer/tegra-timer.c | 130 > > > drivers/usb/host/ehci-tegra.c | 46 +++-- > > > include/configs/tegra-common.h | 6 -- > > > 15 files changed, 274 insertions(+), 79 deletions(-) > > > create mode 100644 drivers/timer/tegra-timer.c > > Ugh... turns out I had completely messed up the testing on Tegra186 and > > it wasn't working at all. The problem is that the selection of the TIMER > > symbol for all of Tegra causes the driver model to be used, but there is > > no DM driver for the architected timer that's used on Tegra186. > > > > The quickest fix would be to do this: > > > > --- >8 --- > > diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig > > index b50eec5b8c9b..05c8ce0e08dd 100644 > > --- a/arch/arm/mach-tegra/Kconfig > > +++ b/arch/arm/mach-tegra/Kconfig > > @@ -56,7 +56,6 @@ config TEGRA_COMMON > > select MISC > > select OF_CONTROL > > select SPI > > - select TIMER > > imply CMD_DM > > imply CRC32_VERIFY > > @@ -83,6 +82,7 @@ config TEGRA_ARMV7_COMMON > > select TEGRA_PINCTRL > > select TEGRA_PMC > > select TEGRA_TIMER > > + select TIMER > > config TEGRA_ARMV8_COMMON > > bool "Tegra 64-bit common options" > > @@ -137,6 +137,7 @@ config TEGRA210 > > select TEGRA_PMC > > select TEGRA_PMC_SECURE > > select TEGRA_TIMER > > + select TIMER > > config TEGRA186 > > bool "Tegra186 family" > > --- >8 --- > > > > So basically make TIMER selected on everything except Tegra186, so that > > on Tegra186 things are basically unmodified. > > I can propose to include 'select TIMER' directly into TEGRA_TIMER config > option. This will eliminate need of including it into mach-tegra Kconfigs. TEGRA_TIMER itself depends on TIMER, which is a more correct dependency given how that's all structured. > BTW, may you check current patch set on T124 and T210 if those work > properly. My T30 devices work fine and same as before after switch to clk_m > as timer calibration clock. With the above on top, the series works fine on Tegra124, Tegra210 and Tegra186. Thierry signature.asc Description: PGP signature
[PATCH 2/2] ARM: tegra: Enable poweroff command on Jetson TX2
From: Thierry Reding This command is useful to power off the system from within U-Boot. Signed-off-by: Thierry Reding --- configs/p2771--000_defconfig | 1 + configs/p2771--500_defconfig | 1 + 2 files changed, 2 insertions(+) diff --git a/configs/p2771--000_defconfig b/configs/p2771--000_defconfig index 682be7d602c9..b769a62efe52 100644 --- a/configs/p2771--000_defconfig +++ b/configs/p2771--000_defconfig @@ -23,6 +23,7 @@ CONFIG_CMD_GPIO=y CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y CONFIG_CMD_PCI=y +CONFIG_CMD_POWEROFF=y CONFIG_CMD_SPI=y # CONFIG_CMD_SETEXPR is not set CONFIG_BOOTP_PREFER_SERVERIP=y diff --git a/configs/p2771--500_defconfig b/configs/p2771--500_defconfig index c5925b18c9f2..421b6b815b0f 100644 --- a/configs/p2771--500_defconfig +++ b/configs/p2771--500_defconfig @@ -23,6 +23,7 @@ CONFIG_CMD_GPIO=y CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y CONFIG_CMD_PCI=y +CONFIG_CMD_POWEROFF=y CONFIG_CMD_SPI=y # CONFIG_CMD_SETEXPR is not set CONFIG_BOOTP_PREFER_SERVERIP=y -- 2.39.1
[PATCH 1/2] ARM: tegra: Enable poweroff command on Jetson TX1 and Jetson Nano
From: Thierry Reding This command is useful to power off the system from within U-Boot. Signed-off-by: Thierry Reding --- configs/p2371-2180_defconfig | 1 + configs/p3450-_defconfig | 1 + 2 files changed, 2 insertions(+) diff --git a/configs/p2371-2180_defconfig b/configs/p2371-2180_defconfig index ea62e18f7384..f99c0bf6e39f 100644 --- a/configs/p2371-2180_defconfig +++ b/configs/p2371-2180_defconfig @@ -27,6 +27,7 @@ CONFIG_CMD_GPIO=y CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y CONFIG_CMD_PCI=y +CONFIG_CMD_POWEROFF=y CONFIG_CMD_SPI=y CONFIG_CMD_USB=y CONFIG_CMD_USB_MASS_STORAGE=y diff --git a/configs/p3450-_defconfig b/configs/p3450-_defconfig index 8e16afde9135..0c5a2835fd69 100644 --- a/configs/p3450-_defconfig +++ b/configs/p3450-_defconfig @@ -28,6 +28,7 @@ CONFIG_CMD_GPIO=y CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y CONFIG_CMD_PCI=y +CONFIG_CMD_POWEROFF=y CONFIG_CMD_SPI=y CONFIG_CMD_USB=y CONFIG_CMD_USB_MASS_STORAGE=y -- 2.39.1
Re: [PATCH v7 0/3] Timer support for ARM Tegra
On Fri, Jan 27, 2023 at 09:13:09AM +0200, Svyatoslav Ryhel wrote: > - ARM: tegra: remap clock_osc_freq for all Tegra family > Enum clock_osc_freq was designed to use only with T20. > This patch remaps it to use additional frequencies, added in > T30+ SoC while maintaining backwards compatibility with T20. > > - drivers: timer: add timer driver for ARMv7 based Tegra devices > Add timer support for T20/T30/T114/T124 and T210 based devices. > Driver is based on DM, has device tree support and can be > used on SPL and early boot stage. > > Arm64 Tegra (apart T210) according to comment in tegra-common.h use > architected timer. > > - ARM: tegra: include timer as default option > Enable TIMER as default option for all Tegra devices and > enable TEGRA_TIMER for TEGRA_ARMV7_COMMON and TEGRA210. > Additionally enable SPL_TIMER if build as SPL part and > drop deprecated configs from common header. > > P. S. I have no arm64 Tegra and according to comment in > tegra-common.h > Use the Tegra US timer on ARMv7, but the architected timer on ARMv8. > > --- > Changeog from V6 > - use clk_m as timer calibration clock (this should properly fix T210) > - enable timer for T210 > > Changed from v5: > - added paz00 tester > > Changed from v4: > - added comments > > Changed from v3: > - removed BOOTSTAGE ifdefs > - use early timer on boot stage unconditionally > --- > Svyatoslav Ryhel (3): > ARM: tegra: remap clock_osc_freq for all Tegra family > drivers: timer: add driver for ARMv7 based Tegra devices and T210 > ARM: tegra: include timer as default option > > arch/arm/Kconfig| 1 + > arch/arm/include/asm/arch-tegra/clock.h | 9 +- > arch/arm/mach-tegra/Kconfig | 3 + > arch/arm/mach-tegra/clock.c | 17 +++- > arch/arm/mach-tegra/cpu.c | 70 ++--- > arch/arm/mach-tegra/tegra114/clock.c| 13 +-- > arch/arm/mach-tegra/tegra124/clock.c| 13 +-- > arch/arm/mach-tegra/tegra20/clock.c | 4 +- > arch/arm/mach-tegra/tegra210/clock.c| 22 +--- > arch/arm/mach-tegra/tegra30/clock.c | 10 +- > drivers/timer/Kconfig | 8 ++ > drivers/timer/Makefile | 1 + > drivers/timer/tegra-timer.c | 130 > drivers/usb/host/ehci-tegra.c | 46 +++-- > include/configs/tegra-common.h | 6 -- > 15 files changed, 274 insertions(+), 79 deletions(-) > create mode 100644 drivers/timer/tegra-timer.c Ugh... turns out I had completely messed up the testing on Tegra186 and it wasn't working at all. The problem is that the selection of the TIMER symbol for all of Tegra causes the driver model to be used, but there is no DM driver for the architected timer that's used on Tegra186. The quickest fix would be to do this: --- >8 --- diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig index b50eec5b8c9b..05c8ce0e08dd 100644 --- a/arch/arm/mach-tegra/Kconfig +++ b/arch/arm/mach-tegra/Kconfig @@ -56,7 +56,6 @@ config TEGRA_COMMON select MISC select OF_CONTROL select SPI - select TIMER imply CMD_DM imply CRC32_VERIFY @@ -83,6 +82,7 @@ config TEGRA_ARMV7_COMMON select TEGRA_PINCTRL select TEGRA_PMC select TEGRA_TIMER + select TIMER config TEGRA_ARMV8_COMMON bool "Tegra 64-bit common options" @@ -137,6 +137,7 @@ config TEGRA210 select TEGRA_PMC select TEGRA_PMC_SECURE select TEGRA_TIMER + select TIMER config TEGRA186 bool "Tegra186 family" --- >8 --- So basically make TIMER selected on everything except Tegra186, so that on Tegra186 things are basically unmodified. Thierry signature.asc Description: PGP signature
Re: [PATCH v6 0/3] Timer support for ARM Tegra
On Thu, Jan 26, 2023 at 07:12:34PM +0200, Svyatoslav Ryhel wrote:
> I may implement changes of Thierry Reding in a proper form as a
> separate patch or include in existing (depends on his choice).
I think it's ultimately better if this is properly integrated into the
series because the series would remain bisectible.
If the existing series is applied as-is, we have a few patches in the
middle during which Tegra210 is unusable. So if we ever have to track
down a regression that might be problematic.
It's not a big deal since we've rarely had regressions in U-Boot. It's
ultimately up to Tom to decide how he wants to handle this.
If you send out another series, can you please add me on Cc so I don't
miss it?
Thanks,
Thierry
> The only thing I need to know is if ALL known T210 devices use
> 19.2MHz as calibration clock for timer?
>
> Best regards.
> Svyatoslav R.
>
> чт, 26 січ. 2023 р. о 18:50 Tom Warren пише:
> >
> > Thanks for testing T210/T186, Thierry.
> >
> > I had Svyatoslav's patches ready to go for a PR yesterday, so I'll need
> > either a patch from you for the T210 changes that I can apply on top of
> > his, or I'll need Svyatoslav to adopt your changes as a 4th patch in his
> > series. Once I have that and can pass buildman OK, I'll be ready to send a
> > PR to TomR.
> >
> > Tom
> >
> > -Original Message-
> > From: Thierry Reding
> > Sent: Thursday, January 26, 2023 4:41 AM
> > To: Svyatoslav Ryhel
> > Cc: Rayagonda Kokatanur ; Tom Warren
> > ; Marek Vasut ; Maxim Schwalm
> > ; Dmitry Osipenko ; Heinrich
> > Schuchardt ; Michal Simek ;
> > Stefan Roese ; Eugen Hristev ;
> > Michael Walle ; Simon Glass ; Jim Liu
> > ; William Zhang ; Rick
> > Chen ; Stefan Herbrechtsmeier
> > ; Andre Przywara
> > ; Jaehoon Chung ;
> > u-boot@lists.denx.de
> > Subject: Re: [PATCH v6 0/3] Timer support for ARM Tegra
> >
> > On Thu, Jan 26, 2023 at 11:34:59AM +0100, Thierry Reding wrote:
> > > On Wed, Jan 25, 2023 at 05:41:08PM +0100, Thierry Reding wrote:
> > > > On Tue, Jan 24, 2023 at 08:57:48AM +0200, Svyatoslav Ryhel wrote:
> > > > > - ARM: tegra: remap clock_osc_freq for all Tegra family Enum
> > > > > clock_osc_freq was designed to use only with T20.
> > > > > This patch remaps it to use additional frequencies, added in
> > > > > T30+ SoC while maintaining backwards compatibility with T20.
> > > > >
> > > > > - drivers: timer: add timer driver for ARMv7 based Tegra devices
> > > > > Add timer support for T20/T30/T114 and T124 based devices.
> > > > > Driver is based on DM, has device tree support and can be used on
> > > > > SPL and early boot stage.
> > > > >
> > > > > - ARM: tegra: include timer as default option Enable TIMER as
> > > > > default option for all Tegra devices and enable TEGRA_TIMER for
> > > > > TEGRA_ARMV7_COMMON. Additionally enable SPL_TIMER if build as SPL
> > > > > part and drop deprecated configs from common header.
> > > > >
> > > > > P. S. I have no arm64 Tegra and according to comment in
> > > > > tegra-common.h Use the Tegra US timer on ARMv7, but the
> > > > > architected timer on ARMv8.
> > > > >
> > > > > Svyatoslav Ryhel (3):
> > > > > ARM: tegra: remap clock_osc_freq for all Tegra family
> > > > > drivers: timer: add timer driver for ARMv7 based Tegra devices
> > > > > ARM: tegra: include timer as default option
> > > >
> > > > This causes a regression on Tegra210 (Jetson TX1). I'm trying to
> > > > investigate, but it's complicated by the fact that I'm not getting
> > > > out any debug prints, so I suspect the issue is happening quite early.
> > >
> > > Alright, I managed to make this work on Tegra210 using the following
> > > patch on top of this series:
> > >
> > > --- >8 ---
> > > diff --git a/arch/arm/dts/tegra210.dtsi b/arch/arm/dts/tegra210.dtsi
> > > index a521a43d6cfd..ccb5a927da89 100644
> > > --- a/arch/arm/dts/tegra210.dtsi
> > > +++ b/arch/arm/dts/tegra210.dtsi
> > > @@ -318,7 +318,7 @@
> > > };
> > >
> > > timer@60005000 {
> > > - compatible = "nvidia,tegra210-timer",
> > > "nvidia,tegra20-timer";
> > > + compatible = "nvidia,tegra210-timer",
> > > "nvidia,tegra30-timer",
> > > +&
Re: [PATCH v6 0/3] Timer support for ARM Tegra
On Thu, Jan 26, 2023 at 07:08:54PM +0200, Svyatoslav Ryhel wrote:
> чт, 26 січ. 2023 р. о 12:35 Thierry Reding пише:
> >
> > On Wed, Jan 25, 2023 at 05:41:08PM +0100, Thierry Reding wrote:
> > > On Tue, Jan 24, 2023 at 08:57:48AM +0200, Svyatoslav Ryhel wrote:
> > > > - ARM: tegra: remap clock_osc_freq for all Tegra family
> > > > Enum clock_osc_freq was designed to use only with T20.
> > > > This patch remaps it to use additional frequencies, added in
> > > > T30+ SoC while maintaining backwards compatibility with T20.
> > > >
> > > > - drivers: timer: add timer driver for ARMv7 based Tegra devices
> > > > Add timer support for T20/T30/T114 and T124 based devices.
> > > > Driver is based on DM, has device tree support and can be
> > > > used on SPL and early boot stage.
> > > >
> > > > - ARM: tegra: include timer as default option
> > > > Enable TIMER as default option for all Tegra devices and
> > > > enable TEGRA_TIMER for TEGRA_ARMV7_COMMON. Additionally
> > > > enable SPL_TIMER if build as SPL part and drop deprecated
> > > > configs from common header.
> > > >
> > > > P. S. I have no arm64 Tegra and according to comment in
> > > > tegra-common.h
> > > > Use the Tegra US timer on ARMv7, but the architected timer on ARMv8.
> > > >
> > > > Svyatoslav Ryhel (3):
> > > > ARM: tegra: remap clock_osc_freq for all Tegra family
> > > > drivers: timer: add timer driver for ARMv7 based Tegra devices
> > > > ARM: tegra: include timer as default option
> > >
> > > This causes a regression on Tegra210 (Jetson TX1). I'm trying to
> > > investigate, but it's complicated by the fact that I'm not getting out
> > > any debug prints, so I suspect the issue is happening quite early.
> >
> > Alright, I managed to make this work on Tegra210 using the following
> > patch on top of this series:
> >
>
> Hello! Thanks for testing this on T210 SoC.
>
> > --- >8 ---
> > diff --git a/arch/arm/dts/tegra210.dtsi b/arch/arm/dts/tegra210.dtsi
> > index a521a43d6cfd..ccb5a927da89 100644
> > --- a/arch/arm/dts/tegra210.dtsi
> > +++ b/arch/arm/dts/tegra210.dtsi
> > @@ -318,7 +318,7 @@
> > };
> >
> > timer@60005000 {
> > - compatible = "nvidia,tegra210-timer",
> > "nvidia,tegra20-timer";
> > + compatible = "nvidia,tegra210-timer",
> > "nvidia,tegra30-timer", "nvidia,tegra20-timer";
>
> This compatibe should not be needed since the driver will have t210
> compatible included.
Yes, it should be fine to leave this as-is. I had included this before
updating the driver, to get the driver to bind to this. Upstream Linux
doesn't include "nvidia,tegra20-timer", so it only has the compatible
string for Tegra210. I think that's slightly better because the register
interface isn't quite compatible. That's a separate issue and we can do
that in a follow-up patch.
>
> > reg = <0x0 0x60005000 0x0 0x400>;
> > interrupts = ,
> > ,
> > diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
> > index cc3f00e50128..b50eec5b8c9b 100644
> > --- a/arch/arm/mach-tegra/Kconfig
> > +++ b/arch/arm/mach-tegra/Kconfig
> > @@ -136,6 +136,7 @@ config TEGRA210
> > select TEGRA_PINCTRL
> > select TEGRA_PMC
> > select TEGRA_PMC_SECURE
> > + select TEGRA_TIMER
> >
> > config TEGRA186
> > bool "Tegra186 family"
> > diff --git a/drivers/timer/tegra-timer.c b/drivers/timer/tegra-timer.c
> > index d2d163cf3fef..235532ba8926 100644
> > --- a/drivers/timer/tegra-timer.c
> > +++ b/drivers/timer/tegra-timer.c
> > @@ -58,17 +58,26 @@ static notrace u64 tegra_timer_get_count(struct udevice
> > *dev)
> > static int tegra_timer_probe(struct udevice *dev)
> > {
> > struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> > + enum clock_osc_freq freq;
> > u32 usec_config, value;
> >
> > /* Timer rate has to be set unconditionally */
> > uc_priv->clock_rate = TEGRA_TIMER_RATE;
> >
> > + /*
> > +* The microsecond timer runs off of clk_m on Tegra210, and clk_m
> > +* runs at half the OSC, so fake this up.
> > +*/
> > + freq = clock_get_osc_freq();
> > + if (freq == CLOCK_OSC
Re: [PATCH v6 0/3] Timer support for ARM Tegra
On Thu, Jan 26, 2023 at 11:34:59AM +0100, Thierry Reding wrote:
> On Wed, Jan 25, 2023 at 05:41:08PM +0100, Thierry Reding wrote:
> > On Tue, Jan 24, 2023 at 08:57:48AM +0200, Svyatoslav Ryhel wrote:
> > > - ARM: tegra: remap clock_osc_freq for all Tegra family
> > > Enum clock_osc_freq was designed to use only with T20.
> > > This patch remaps it to use additional frequencies, added in
> > > T30+ SoC while maintaining backwards compatibility with T20.
> > >
> > > - drivers: timer: add timer driver for ARMv7 based Tegra devices
> > > Add timer support for T20/T30/T114 and T124 based devices.
> > > Driver is based on DM, has device tree support and can be
> > > used on SPL and early boot stage.
> > >
> > > - ARM: tegra: include timer as default option
> > > Enable TIMER as default option for all Tegra devices and
> > > enable TEGRA_TIMER for TEGRA_ARMV7_COMMON. Additionally
> > > enable SPL_TIMER if build as SPL part and drop deprecated
> > > configs from common header.
> > >
> > > P. S. I have no arm64 Tegra and according to comment in
> > > tegra-common.h
> > > Use the Tegra US timer on ARMv7, but the architected timer on ARMv8.
> > >
> > > Svyatoslav Ryhel (3):
> > > ARM: tegra: remap clock_osc_freq for all Tegra family
> > > drivers: timer: add timer driver for ARMv7 based Tegra devices
> > > ARM: tegra: include timer as default option
> >
> > This causes a regression on Tegra210 (Jetson TX1). I'm trying to
> > investigate, but it's complicated by the fact that I'm not getting out
> > any debug prints, so I suspect the issue is happening quite early.
>
> Alright, I managed to make this work on Tegra210 using the following
> patch on top of this series:
>
> --- >8 ---
> diff --git a/arch/arm/dts/tegra210.dtsi b/arch/arm/dts/tegra210.dtsi
> index a521a43d6cfd..ccb5a927da89 100644
> --- a/arch/arm/dts/tegra210.dtsi
> +++ b/arch/arm/dts/tegra210.dtsi
> @@ -318,7 +318,7 @@
> };
>
> timer@60005000 {
> - compatible = "nvidia,tegra210-timer", "nvidia,tegra20-timer";
> + compatible = "nvidia,tegra210-timer", "nvidia,tegra30-timer",
> "nvidia,tegra20-timer";
> reg = <0x0 0x60005000 0x0 0x400>;
> interrupts = ,
>,
> diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
> index cc3f00e50128..b50eec5b8c9b 100644
> --- a/arch/arm/mach-tegra/Kconfig
> +++ b/arch/arm/mach-tegra/Kconfig
> @@ -136,6 +136,7 @@ config TEGRA210
> select TEGRA_PINCTRL
> select TEGRA_PMC
> select TEGRA_PMC_SECURE
> + select TEGRA_TIMER
>
> config TEGRA186
> bool "Tegra186 family"
> diff --git a/drivers/timer/tegra-timer.c b/drivers/timer/tegra-timer.c
> index d2d163cf3fef..235532ba8926 100644
> --- a/drivers/timer/tegra-timer.c
> +++ b/drivers/timer/tegra-timer.c
> @@ -58,17 +58,26 @@ static notrace u64 tegra_timer_get_count(struct udevice
> *dev)
> static int tegra_timer_probe(struct udevice *dev)
> {
> struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> + enum clock_osc_freq freq;
> u32 usec_config, value;
>
> /* Timer rate has to be set unconditionally */
> uc_priv->clock_rate = TEGRA_TIMER_RATE;
>
> + /*
> + * The microsecond timer runs off of clk_m on Tegra210, and clk_m
> + * runs at half the OSC, so fake this up.
> + */
> + freq = clock_get_osc_freq();
> + if (freq == CLOCK_OSC_FREQ_38_4)
> + freq = CLOCK_OSC_FREQ_19_2;
> +
> /*
>* Configure microsecond timers to have 1MHz clock
>* Config register is 0xqqww, where qq is "dividend", ww is "divisor"
>* Uses n+1 scheme
>*/
> - switch (clock_get_osc_freq()) {
> + switch (freq) {
> case CLOCK_OSC_FREQ_13_0:
> usec_config = 0x000c; /* (12+1)/(0+1) */
> break;
> @@ -113,6 +122,7 @@ static const struct udevice_id tegra_timer_ids[] = {
> { .compatible = "nvidia,tegra30-timer" },
> { .compatible = "nvidia,tegra114-timer" },
> { .compatible = "nvidia,tegra124-timer" },
> + { .compatible = "nvidia,tegra210-timer" },
> { }
> };
> --- >8 ---
>
> I've also tested this on Tegra186, though no additional changes were
> needed since Tegra186 doesn't use the Tegra timer.
>
> With the above folded in, the series is:
>
> Tested-by: Thierry Reding
I've also tested your series with the above on Tegra30 (Beaver) and
Tegra124 (Jetson TK1), both seem to work fine.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v6 0/3] Timer support for ARM Tegra
On Wed, Jan 25, 2023 at 05:41:08PM +0100, Thierry Reding wrote:
> On Tue, Jan 24, 2023 at 08:57:48AM +0200, Svyatoslav Ryhel wrote:
> > - ARM: tegra: remap clock_osc_freq for all Tegra family
> > Enum clock_osc_freq was designed to use only with T20.
> > This patch remaps it to use additional frequencies, added in
> > T30+ SoC while maintaining backwards compatibility with T20.
> >
> > - drivers: timer: add timer driver for ARMv7 based Tegra devices
> > Add timer support for T20/T30/T114 and T124 based devices.
> > Driver is based on DM, has device tree support and can be
> > used on SPL and early boot stage.
> >
> > - ARM: tegra: include timer as default option
> > Enable TIMER as default option for all Tegra devices and
> > enable TEGRA_TIMER for TEGRA_ARMV7_COMMON. Additionally
> > enable SPL_TIMER if build as SPL part and drop deprecated
> > configs from common header.
> >
> > P. S. I have no arm64 Tegra and according to comment in
> > tegra-common.h
> > Use the Tegra US timer on ARMv7, but the architected timer on ARMv8.
> >
> > Svyatoslav Ryhel (3):
> > ARM: tegra: remap clock_osc_freq for all Tegra family
> > drivers: timer: add timer driver for ARMv7 based Tegra devices
> > ARM: tegra: include timer as default option
>
> This causes a regression on Tegra210 (Jetson TX1). I'm trying to
> investigate, but it's complicated by the fact that I'm not getting out
> any debug prints, so I suspect the issue is happening quite early.
Alright, I managed to make this work on Tegra210 using the following
patch on top of this series:
--- >8 ---
diff --git a/arch/arm/dts/tegra210.dtsi b/arch/arm/dts/tegra210.dtsi
index a521a43d6cfd..ccb5a927da89 100644
--- a/arch/arm/dts/tegra210.dtsi
+++ b/arch/arm/dts/tegra210.dtsi
@@ -318,7 +318,7 @@
};
timer@60005000 {
- compatible = "nvidia,tegra210-timer", "nvidia,tegra20-timer";
+ compatible = "nvidia,tegra210-timer", "nvidia,tegra30-timer",
"nvidia,tegra20-timer";
reg = <0x0 0x60005000 0x0 0x400>;
interrupts = ,
,
diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index cc3f00e50128..b50eec5b8c9b 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -136,6 +136,7 @@ config TEGRA210
select TEGRA_PINCTRL
select TEGRA_PMC
select TEGRA_PMC_SECURE
+ select TEGRA_TIMER
config TEGRA186
bool "Tegra186 family"
diff --git a/drivers/timer/tegra-timer.c b/drivers/timer/tegra-timer.c
index d2d163cf3fef..235532ba8926 100644
--- a/drivers/timer/tegra-timer.c
+++ b/drivers/timer/tegra-timer.c
@@ -58,17 +58,26 @@ static notrace u64 tegra_timer_get_count(struct udevice
*dev)
static int tegra_timer_probe(struct udevice *dev)
{
struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+ enum clock_osc_freq freq;
u32 usec_config, value;
/* Timer rate has to be set unconditionally */
uc_priv->clock_rate = TEGRA_TIMER_RATE;
+ /*
+* The microsecond timer runs off of clk_m on Tegra210, and clk_m
+* runs at half the OSC, so fake this up.
+*/
+ freq = clock_get_osc_freq();
+ if (freq == CLOCK_OSC_FREQ_38_4)
+ freq = CLOCK_OSC_FREQ_19_2;
+
/*
* Configure microsecond timers to have 1MHz clock
* Config register is 0xqqww, where qq is "dividend", ww is "divisor"
* Uses n+1 scheme
*/
- switch (clock_get_osc_freq()) {
+ switch (freq) {
case CLOCK_OSC_FREQ_13_0:
usec_config = 0x000c; /* (12+1)/(0+1) */
break;
@@ -113,6 +122,7 @@ static const struct udevice_id tegra_timer_ids[] = {
{ .compatible = "nvidia,tegra30-timer" },
{ .compatible = "nvidia,tegra114-timer" },
{ .compatible = "nvidia,tegra124-timer" },
+ { .compatible = "nvidia,tegra210-timer" },
{ }
};
--- >8 ---
I've also tested this on Tegra186, though no additional changes were
needed since Tegra186 doesn't use the Tegra timer.
With the above folded in, the series is:
Tested-by: Thierry Reding
signature.asc
Description: PGP signature
Re: [PATCH v6 0/3] Timer support for ARM Tegra
On Tue, Jan 24, 2023 at 08:57:48AM +0200, Svyatoslav Ryhel wrote: > - ARM: tegra: remap clock_osc_freq for all Tegra family > Enum clock_osc_freq was designed to use only with T20. > This patch remaps it to use additional frequencies, added in > T30+ SoC while maintaining backwards compatibility with T20. > > - drivers: timer: add timer driver for ARMv7 based Tegra devices > Add timer support for T20/T30/T114 and T124 based devices. > Driver is based on DM, has device tree support and can be > used on SPL and early boot stage. > > - ARM: tegra: include timer as default option > Enable TIMER as default option for all Tegra devices and > enable TEGRA_TIMER for TEGRA_ARMV7_COMMON. Additionally > enable SPL_TIMER if build as SPL part and drop deprecated > configs from common header. > > P. S. I have no arm64 Tegra and according to comment in > tegra-common.h > Use the Tegra US timer on ARMv7, but the architected timer on ARMv8. > > Svyatoslav Ryhel (3): > ARM: tegra: remap clock_osc_freq for all Tegra family > drivers: timer: add timer driver for ARMv7 based Tegra devices > ARM: tegra: include timer as default option This causes a regression on Tegra210 (Jetson TX1). I'm trying to investigate, but it's complicated by the fact that I'm not getting out any debug prints, so I suspect the issue is happening quite early. Thierry > > arch/arm/Kconfig| 1 + > arch/arm/include/asm/arch-tegra/clock.h | 9 +- > arch/arm/mach-tegra/Kconfig | 2 + > arch/arm/mach-tegra/clock.c | 17 +++- > arch/arm/mach-tegra/cpu.c | 70 ++--- > arch/arm/mach-tegra/tegra114/clock.c| 13 +-- > arch/arm/mach-tegra/tegra124/clock.c| 13 +-- > arch/arm/mach-tegra/tegra20/clock.c | 4 +- > arch/arm/mach-tegra/tegra210/clock.c| 22 + > arch/arm/mach-tegra/tegra30/clock.c | 10 +- > drivers/timer/Kconfig | 8 ++ > drivers/timer/Makefile | 1 + > drivers/timer/tegra-timer.c | 126 > drivers/usb/host/ehci-tegra.c | 46 +++-- > include/configs/tegra-common.h | 6 -- > 15 files changed, 269 insertions(+), 79 deletions(-) > create mode 100644 drivers/timer/tegra-timer.c > > -- > 2.37.2 > signature.asc Description: PGP signature
[PATCH 8/9] ARM: tegra: Refactor DT update helpers
From: Thierry Reding
Rather than duplicate the Ethernet MAC address and carveout updating
code for each board, move it to a common location and make it more
reusable.
Signed-off-by: Thierry Reding
---
arch/arm/include/asm/arch-tegra/board.h | 10 ++
arch/arm/mach-tegra/dt-setup.c | 118 ++
board/nvidia/p2371-2180/p2371-2180.c| 123 ++-
board/nvidia/p2771-/p2771-.c| 126 ++-
board/nvidia/p3450-/p3450-.c| 127 ++--
5 files changed, 149 insertions(+), 355 deletions(-)
diff --git a/arch/arm/include/asm/arch-tegra/board.h
b/arch/arm/include/asm/arch-tegra/board.h
index 24d0db8ced83..cd4d0ee3c953 100644
--- a/arch/arm/include/asm/arch-tegra/board.h
+++ b/arch/arm/include/asm/arch-tegra/board.h
@@ -30,4 +30,14 @@ void pin_mux_nand(void); /* overridable NAND pinmux
setup*/
void pin_mux_mmc(void); /* overridable mmc pinmux setup */
void pin_mux_display(void); /* overridable DISPLAY pinmux setup */
+/*
+ * Helpers for various standard DT update mechanisms.
+ */
+
+#if defined(CONFIG_ARM64)
+void ft_mac_address_setup(void *fdt);
+void ft_carveout_setup(void *fdt, const char *const *nodes,
+ unsigned int count);
+#endif
+
#endif
diff --git a/arch/arm/mach-tegra/dt-setup.c b/arch/arm/mach-tegra/dt-setup.c
index 602b20e6b7e9..894a6358a2c4 100644
--- a/arch/arm/mach-tegra/dt-setup.c
+++ b/arch/arm/mach-tegra/dt-setup.c
@@ -4,6 +4,9 @@
*/
#include
+#include
+#include
+#include
#include
/*
@@ -31,3 +34,118 @@ int ft_system_setup(void *blob, struct bd_info *bd)
return 0;
}
+
+#if defined(CONFIG_ARM64)
+void ft_mac_address_setup(void *fdt)
+{
+ const void *cboot_fdt = (const void *)cboot_boot_x0;
+ uint8_t mac[ETH_ALEN], local_mac[ETH_ALEN];
+ const char *path;
+ int offset, err;
+
+ err = cboot_get_ethaddr(cboot_fdt, local_mac);
+ if (err < 0)
+ memset(local_mac, 0, ETH_ALEN);
+
+ path = fdt_get_alias(fdt, "ethernet");
+ if (!path)
+ return;
+
+ debug("ethernet alias found: %s\n", path);
+
+ offset = fdt_path_offset(fdt, path);
+ if (offset < 0) {
+ printf("ethernet alias points to absent node %s\n", path);
+ return;
+ }
+
+ if (is_valid_ethaddr(local_mac)) {
+ err = fdt_setprop(fdt, offset, "local-mac-address", local_mac,
+ ETH_ALEN);
+ if (!err)
+ debug("Local MAC address set: %pM\n", local_mac);
+ }
+
+ if (eth_env_get_enetaddr("ethaddr", mac)) {
+ if (memcmp(local_mac, mac, ETH_ALEN) != 0) {
+ err = fdt_setprop(fdt, offset, "mac-address", mac,
+ ETH_ALEN);
+ if (!err)
+ debug("MAC address set: %pM\n", mac);
+ }
+ }
+}
+
+static int ft_copy_carveout(void *dst, const void *src, const char *node)
+{
+ struct fdt_memory carveout;
+ unsigned int index = 0;
+ int err;
+
+ while (true) {
+ const char **compatibles = NULL;
+ unsigned int num_compatibles;
+ unsigned long flags;
+ char *copy = NULL;
+ const char *name;
+
+ err = fdtdec_get_carveout(src, node, "memory-region", index,
+ , , ,
+ _compatibles, );
+ if (err < 0) {
+ if (err != -FDT_ERR_NOTFOUND)
+ printf("failed to get carveout for %s: %d\n",
+ node, err);
+
+ return err;
+ }
+
+ if (name) {
+ const char *ptr = strchr(name, '@');
+
+ if (ptr) {
+ copy = strndup(name, ptr - name);
+ name = copy;
+ }
+ } else {
+ name = "carveout";
+ }
+
+ err = fdtdec_set_carveout(dst, node, "memory-region", index,
+ , name, compatibles,
+ num_compatibles, flags);
+ if (err < 0) {
+ printf("failed to set carveout for %s: %d\n", node,
+ err);
+ return err;
+ }
+
+ if (copy)
+ free(copy);
+
+ index++;
+ }
+
+ return 0;
+}
+
+void ft_carveout_setup(void *fdt, const char * const *nodes, unsigned int
coun
[PATCH 5/9] fdtdec: Support reserved-memory flags
From: Thierry Reding
Reserved memory nodes can have additional flags. Support reading and
writing these flags to ensure that reserved memory nodes can be properly
parsed and emitted.
This converts support for the existing "no-map" flag to avoid extending
the argument list for fdtdec_add_reserved_memory() to excessive length.
Signed-off-by: Thierry Reding
---
arch/arm/cpu/armv8/fsl-layerscape/soc.c | 2 +-
arch/riscv/lib/fdt_fixup.c | 2 +-
board/nvidia/p2371-2180/p2371-2180.c| 4 ++--
board/nvidia/p2771-/p2771-.c| 4 ++--
board/nvidia/p3450-/p3450-.c| 4 ++--
include/fdtdec.h| 20 +++---
lib/fdtdec.c| 28 -
lib/fdtdec_test.c | 4 ++--
lib/optee/optee.c | 3 ++-
test/dm/fdtdec.c| 13 ++--
10 files changed, 50 insertions(+), 34 deletions(-)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
index faac618dc48a..6cebd8259a58 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
@@ -55,7 +55,7 @@ int ls_gic_rd_tables_init(void *blob)
lpi_base.start = addr;
lpi_base.end = addr + size - 1;
ret = fdtdec_add_reserved_memory(blob, "lpi_rd_table", _base, NULL,
-NULL, 0, false);
+NULL, 0, 0);
if (ret) {
debug("%s: failed to add reserved memory\n", __func__);
return ret;
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c
index 96c78baeb47d..8858b1bac870 100644
--- a/arch/riscv/lib/fdt_fixup.c
+++ b/arch/riscv/lib/fdt_fixup.c
@@ -76,7 +76,7 @@ int riscv_fdt_copy_resv_mem_node(const void *src, void *dst)
pmp_mem.start = addr;
pmp_mem.end = addr + size - 1;
err = fdtdec_add_reserved_memory(dst, basename, _mem,
-NULL, 0, , false);
+NULL, 0, , 0);
if (err < 0 && err != -FDT_ERR_EXISTS) {
log_err("failed to add reserved memory: %d\n", err);
return err;
diff --git a/board/nvidia/p2371-2180/p2371-2180.c
b/board/nvidia/p2371-2180/p2371-2180.c
index bc0a133725ed..137c7d3b12c3 100644
--- a/board/nvidia/p2371-2180/p2371-2180.c
+++ b/board/nvidia/p2371-2180/p2371-2180.c
@@ -129,7 +129,7 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
int err;
err = fdtdec_get_carveout(src, node, "memory-region", 0, , NULL,
- NULL, NULL);
+ NULL, NULL, NULL);
if (err < 0) {
if (err != -FDT_ERR_NOTFOUND)
printf("failed to get carveout for %s: %d\n", node,
@@ -139,7 +139,7 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
}
err = fdtdec_set_carveout(dst, node, "memory-region", 0, ,
- "framebuffer", NULL, 0);
+ "framebuffer", NULL, 0, 0);
if (err < 0) {
printf("failed to set carveout for %s: %d\n", node, err);
return err;
diff --git a/board/nvidia/p2771-/p2771-.c
b/board/nvidia/p2771-/p2771-.c
index cde5eff02f2a..3d2653d1f075 100644
--- a/board/nvidia/p2771-/p2771-.c
+++ b/board/nvidia/p2771-/p2771-.c
@@ -105,7 +105,7 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
int err;
err = fdtdec_get_carveout(src, node, "memory-region", 0, , NULL,
- NULL, NULL);
+ NULL, NULL, NULL);
if (err < 0) {
if (err != -FDT_ERR_NOTFOUND)
printf("failed to get carveout for %s: %d\n", node,
@@ -115,7 +115,7 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
}
err = fdtdec_set_carveout(dst, node, "memory-region", 0, ,
- "framebuffer", NULL, 0);
+ "framebuffer", NULL, 0, 0);
if (err < 0) {
printf("failed to set carveout for %s: %d\n", node, err);
return err;
diff --git a/board/nvidia/p3450-/p3450-.c
b/board/nvidia/p3450-/p3450-.c
index 541863cef361..cdedea18a176 100644
--- a/board/nvidia/p3450-/p3450-.c
+++ b/board/nvidia/p3450-/p3450-.c
@@ -129,7 +129,7 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
int err;
[PATCH 6/9] ARM: tegra: Support multiple reserved memory regions
From: Thierry Reding
Support multiple reserved memory regions per device to support platforms
that use both a framebuffer and color conversion lookup table for early
boot display splash.
While at it, also pass along the name, compatible strings and flags of
the carveouts.
Signed-off-by: Thierry Reding
---
board/nvidia/p2371-2180/p2371-2180.c | 55 +---
board/nvidia/p2771-/p2771-.c | 55 +---
board/nvidia/p3450-/p3450-.c | 55 +---
3 files changed, 126 insertions(+), 39 deletions(-)
diff --git a/board/nvidia/p2371-2180/p2371-2180.c
b/board/nvidia/p2371-2180/p2371-2180.c
index 137c7d3b12c3..f5126c552b00 100644
--- a/board/nvidia/p2371-2180/p2371-2180.c
+++ b/board/nvidia/p2371-2180/p2371-2180.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -125,24 +126,52 @@ static void ft_mac_address_setup(void *fdt)
static int ft_copy_carveout(void *dst, const void *src, const char *node)
{
- struct fdt_memory fb;
+ unsigned int index = 0;
int err;
- err = fdtdec_get_carveout(src, node, "memory-region", 0, , NULL,
- NULL, NULL, NULL);
- if (err < 0) {
- if (err != -FDT_ERR_NOTFOUND)
- printf("failed to get carveout for %s: %d\n", node,
+ while (true) {
+ const char **compatibles = NULL;
+ unsigned int num_compatibles;
+ struct fdt_memory carveout;
+ unsigned long flags;
+ char *copy = NULL;
+ const char *name;
+
+ err = fdtdec_get_carveout(src, node, "memory-region", index,
+ , , ,
+ _compatibles, );
+ if (err < 0) {
+ if (err != -FDT_ERR_NOTFOUND)
+ printf("failed to get carveout for %s: %d\n",
+ node, err);
+
+ return err;
+ }
+
+ if (name) {
+ const char *ptr = strchr(name, '@');
+
+ if (ptr) {
+ copy = strndup(name, ptr - name);
+ name = copy;
+ }
+ } else {
+ name = "carveout";
+ }
+
+ err = fdtdec_set_carveout(dst, node, "memory-region", index,
+ , name, compatibles,
+ num_compatibles, flags);
+ if (err < 0) {
+ printf("failed to set carveout for %s: %d\n", node,
err);
+ return err;
+ }
- return err;
- }
+ if (copy)
+ free(copy);
- err = fdtdec_set_carveout(dst, node, "memory-region", 0, ,
- "framebuffer", NULL, 0, 0);
- if (err < 0) {
- printf("failed to set carveout for %s: %d\n", node, err);
- return err;
+ index++;
}
return 0;
diff --git a/board/nvidia/p2771-/p2771-.c
b/board/nvidia/p2771-/p2771-.c
index 3d2653d1f075..46c36a22db5e 100644
--- a/board/nvidia/p2771-/p2771-.c
+++ b/board/nvidia/p2771-/p2771-.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
#include "../p2571/max77620_init.h"
@@ -101,24 +102,52 @@ static void ft_mac_address_setup(void *fdt)
static int ft_copy_carveout(void *dst, const void *src, const char *node)
{
- struct fdt_memory fb;
+ unsigned int index = 0;
int err;
- err = fdtdec_get_carveout(src, node, "memory-region", 0, , NULL,
- NULL, NULL, NULL);
- if (err < 0) {
- if (err != -FDT_ERR_NOTFOUND)
- printf("failed to get carveout for %s: %d\n", node,
+ while (true) {
+ const char **compatibles = NULL;
+ unsigned int num_compatibles;
+ struct fdt_memory carveout;
+ unsigned long flags;
+ char *copy = NULL;
+ const char *name;
+
+ err = fdtdec_get_carveout(src, node, "memory-region", index,
+ , , ,
+ _compatibles, );
+ if (err < 0) {
+ if (err != -FDT_ERR_NOTFOUND)
+ printf("failed to get carveout for %s: %d\n",
+ node, err);
+
+
[PATCH 9/9] ARM: tegra: Copy memory-region-names property
From: Thierry Reding
If multiple entries are present in the memory-region property, this new
memory-region-names property can be used to specify names for each of
them so that they can be more easily distinguished.
Signed-off-by: Thierry Reding
---
arch/arm/mach-tegra/dt-setup.c | 31 ++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-tegra/dt-setup.c b/arch/arm/mach-tegra/dt-setup.c
index 894a6358a2c4..c11494722bc7 100644
--- a/arch/arm/mach-tegra/dt-setup.c
+++ b/arch/arm/mach-tegra/dt-setup.c
@@ -78,9 +78,11 @@ void ft_mac_address_setup(void *fdt)
static int ft_copy_carveout(void *dst, const void *src, const char *node)
{
+ const char *names = "memory-region-names";
struct fdt_memory carveout;
unsigned int index = 0;
- int err;
+ int err, offset, len;
+ const void *prop;
while (true) {
const char **compatibles = NULL;
@@ -96,6 +98,8 @@ static int ft_copy_carveout(void *dst, const void *src, const
char *node)
if (err != -FDT_ERR_NOTFOUND)
printf("failed to get carveout for %s: %d\n",
node, err);
+ else
+ break;
return err;
}
@@ -126,6 +130,31 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
index++;
}
+ offset = fdt_path_offset(src, node);
+ if (offset < 0) {
+ debug("failed to find source offset for %s: %s\n", node,
+ fdt_strerror(err));
+ return err;
+ }
+
+ prop = fdt_getprop(src, offset, names, );
+ if (prop) {
+ offset = fdt_path_offset(dst, node);
+ if (offset < 0) {
+ debug("failed to find destination offset for %s: %s\n",
+ node, fdt_strerror(err));
+ return err;
+ }
+
+ err = fdt_setprop(dst, offset, "memory-region-names", prop,
+ len);
+ if (err < 0) {
+ debug("failed to copy \"%s\" property: %s\n", names,
+ fdt_strerror(err));
+ return err;
+ }
+ }
+
return 0;
}
--
2.33.0
[PATCH 7/9] ARM: tegra: Support EMC frequency tables on Tegra210
From: Thierry Reding
The EMC frequency tables are created from a training sequence performed
during early boot and passed in via a reserved memory region by nvtboot.
Copy this table to the kernel DTB so that the kernel can use it to scale
the EMC frequency at runtime.
Note that early bootloaders store the EMC table at an address that
currently intersects with the load address of the initial ramdisk. In
order to avoid copying the table to a different address, simply change
the load address for the initial ramdisk in U-Boot.
Signed-off-by: Thierry Reding
---
board/nvidia/p2371-2180/p2371-2180.c | 1 +
board/nvidia/p3450-/p3450-.c | 1 +
include/configs/tegra210-common.h| 2 +-
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/board/nvidia/p2371-2180/p2371-2180.c
b/board/nvidia/p2371-2180/p2371-2180.c
index f5126c552b00..cd5dc2de629e 100644
--- a/board/nvidia/p2371-2180/p2371-2180.c
+++ b/board/nvidia/p2371-2180/p2371-2180.c
@@ -183,6 +183,7 @@ static void ft_carveout_setup(void *fdt)
static const char * const nodes[] = {
"/host1x@5000/dc@5420",
"/host1x@5000/dc@5424",
+ "/external-memory-controller@7001b000",
};
unsigned int i;
int err;
diff --git a/board/nvidia/p3450-/p3450-.c
b/board/nvidia/p3450-/p3450-.c
index 2c709d9b8117..dd408d2ebbf8 100644
--- a/board/nvidia/p3450-/p3450-.c
+++ b/board/nvidia/p3450-/p3450-.c
@@ -183,6 +183,7 @@ static void ft_carveout_setup(void *fdt)
static const char * const nodes[] = {
"/host1x@5000/dc@5420",
"/host1x@5000/dc@5424",
+ "/external-memory-controller@7001b000",
};
unsigned int i;
int err;
diff --git a/include/configs/tegra210-common.h
b/include/configs/tegra210-common.h
index 2226effe16ab..c38d0f831835 100644
--- a/include/configs/tegra210-common.h
+++ b/include/configs/tegra210-common.h
@@ -48,7 +48,7 @@
"kernel_addr_r=" __stringify(CONFIG_LOADADDR) "\0" \
"fdtfile=" FDTFILE "\0" \
"fdt_addr_r=0x8300\0" \
- "ramdisk_addr_r=0x8320\0"
+ "ramdisk_addr_r=0x8342\0"
/* For USB EHCI controller */
#define CONFIG_EHCI_IS_TDI
--
2.33.0
[PATCH 3/9] fdtdec: Support compatible string list for reserved memory
From: Thierry Reding
Reserved memory nodes can have a compatible string list to identify the
type of reserved memory that they represent. Support specifying an
optional compatible string list when creating these nodes.
Signed-off-by: Thierry Reding
---
arch/arm/cpu/armv8/fsl-layerscape/soc.c | 3 +-
arch/riscv/lib/fdt_fixup.c | 2 +-
board/nvidia/p2371-2180/p2371-2180.c| 5 +-
board/nvidia/p2771-/p2771-.c| 5 +-
board/nvidia/p3450-/p3450-.c| 5 +-
include/fdtdec.h| 17 --
lib/fdtdec.c| 69 -
lib/fdtdec_test.c | 4 +-
lib/optee/optee.c | 1 +
test/dm/fdtdec.c| 18 +++
10 files changed, 105 insertions(+), 24 deletions(-)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
index 42a096854629..faac618dc48a 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
@@ -54,7 +54,8 @@ int ls_gic_rd_tables_init(void *blob)
lpi_base.start = addr;
lpi_base.end = addr + size - 1;
- ret = fdtdec_add_reserved_memory(blob, "lpi_rd_table", _base, NULL,
false);
+ ret = fdtdec_add_reserved_memory(blob, "lpi_rd_table", _base, NULL,
+NULL, 0, false);
if (ret) {
debug("%s: failed to add reserved memory\n", __func__);
return ret;
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c
index f636b2844978..96c78baeb47d 100644
--- a/arch/riscv/lib/fdt_fixup.c
+++ b/arch/riscv/lib/fdt_fixup.c
@@ -76,7 +76,7 @@ int riscv_fdt_copy_resv_mem_node(const void *src, void *dst)
pmp_mem.start = addr;
pmp_mem.end = addr + size - 1;
err = fdtdec_add_reserved_memory(dst, basename, _mem,
-, false);
+NULL, 0, , false);
if (err < 0 && err != -FDT_ERR_EXISTS) {
log_err("failed to add reserved memory: %d\n", err);
return err;
diff --git a/board/nvidia/p2371-2180/p2371-2180.c
b/board/nvidia/p2371-2180/p2371-2180.c
index 1f7aa0050cde..58077255d073 100644
--- a/board/nvidia/p2371-2180/p2371-2180.c
+++ b/board/nvidia/p2371-2180/p2371-2180.c
@@ -128,7 +128,8 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
struct fdt_memory fb;
int err;
- err = fdtdec_get_carveout(src, node, "memory-region", 0, , NULL);
+ err = fdtdec_get_carveout(src, node, "memory-region", 0, , NULL,
+ NULL, NULL);
if (err < 0) {
if (err != -FDT_ERR_NOTFOUND)
printf("failed to get carveout for %s: %d\n", node,
@@ -138,7 +139,7 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
}
err = fdtdec_set_carveout(dst, node, "memory-region", 0, "framebuffer",
- );
+ NULL, 0, );
if (err < 0) {
printf("failed to set carveout for %s: %d\n", node, err);
return err;
diff --git a/board/nvidia/p2771-/p2771-.c
b/board/nvidia/p2771-/p2771-.c
index aca86c342680..e35e6b6f48dc 100644
--- a/board/nvidia/p2771-/p2771-.c
+++ b/board/nvidia/p2771-/p2771-.c
@@ -104,7 +104,8 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
struct fdt_memory fb;
int err;
- err = fdtdec_get_carveout(src, node, "memory-region", 0, , NULL);
+ err = fdtdec_get_carveout(src, node, "memory-region", 0, , NULL,
+ NULL, NULL);
if (err < 0) {
if (err != -FDT_ERR_NOTFOUND)
printf("failed to get carveout for %s: %d\n", node,
@@ -114,7 +115,7 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
}
err = fdtdec_set_carveout(dst, node, "memory-region", 0, "framebuffer",
- );
+ NULL, 0, );
if (err < 0) {
printf("failed to set carveout for %s: %d\n", node, err);
return err;
diff --git a/board/nvidia/p3450-/p3450-.c
b/board/nvidia/p3450-/p3450-.c
index 132eb824c675..d9ef45af5eea 100644
--- a/board/nvidia/p3450-/p3450-.c
+++ b/board/nvidia/p3450-/p3450-.c
@@ -128,7 +128,8 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
struct fdt_memory fb;
int err;
- err = fdtdec_get_carve
[PATCH 4/9] fdtdec: Reorder fdtdec_set_carveout() parameters for consistency
From: Thierry Reding
The fdtdec_set_carveout() function's parameters are inconsistent with
the parameters passed to fdtdec_add_reserved_memory(). Fix up the order
to make it more consistent.
Signed-off-by: Thierry Reding
---
board/nvidia/p2371-2180/p2371-2180.c | 4 ++--
board/nvidia/p2771-/p2771-.c | 4 ++--
board/nvidia/p3450-/p3450-.c | 4 ++--
include/fdtdec.h | 8
lib/fdtdec.c | 6 +++---
lib/fdtdec_test.c| 4 ++--
test/dm/fdtdec.c | 15 ++-
7 files changed, 21 insertions(+), 24 deletions(-)
diff --git a/board/nvidia/p2371-2180/p2371-2180.c
b/board/nvidia/p2371-2180/p2371-2180.c
index 58077255d073..bc0a133725ed 100644
--- a/board/nvidia/p2371-2180/p2371-2180.c
+++ b/board/nvidia/p2371-2180/p2371-2180.c
@@ -138,8 +138,8 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
return err;
}
- err = fdtdec_set_carveout(dst, node, "memory-region", 0, "framebuffer",
- NULL, 0, );
+ err = fdtdec_set_carveout(dst, node, "memory-region", 0, ,
+ "framebuffer", NULL, 0);
if (err < 0) {
printf("failed to set carveout for %s: %d\n", node, err);
return err;
diff --git a/board/nvidia/p2771-/p2771-.c
b/board/nvidia/p2771-/p2771-.c
index e35e6b6f48dc..cde5eff02f2a 100644
--- a/board/nvidia/p2771-/p2771-.c
+++ b/board/nvidia/p2771-/p2771-.c
@@ -114,8 +114,8 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
return err;
}
- err = fdtdec_set_carveout(dst, node, "memory-region", 0, "framebuffer",
- NULL, 0, );
+ err = fdtdec_set_carveout(dst, node, "memory-region", 0, ,
+ "framebuffer", NULL, 0);
if (err < 0) {
printf("failed to set carveout for %s: %d\n", node, err);
return err;
diff --git a/board/nvidia/p3450-/p3450-.c
b/board/nvidia/p3450-/p3450-.c
index d9ef45af5eea..541863cef361 100644
--- a/board/nvidia/p3450-/p3450-.c
+++ b/board/nvidia/p3450-/p3450-.c
@@ -138,8 +138,8 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
return err;
}
- err = fdtdec_set_carveout(dst, node, "memory-region", 0, "framebuffer",
- NULL, 0, );
+ err = fdtdec_set_carveout(dst, node, "memory-region", 0, ,
+ "framebuffer", NULL, 0);
if (err < 0) {
printf("failed to set carveout for %s: %d\n", node, err);
return err;
diff --git a/include/fdtdec.h b/include/fdtdec.h
index 489f5063763b..6d56c67d111c 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -1143,16 +1143,16 @@ int fdtdec_get_carveout(const void *blob, const char
*node,
* @param prop_namename of the property in which to store the phandle of
* the carveout
* @param indexindex of the phandle to store
- * @param name base name of the reserved-memory node to create
* @param carveout information about the carveout to add
+ * @param name base name of the reserved-memory node to create
* @param compatibles compatible strings to set for the carveout
* @param countnumber of compatible strings
* @return 0 on success or a negative error code on failure
*/
int fdtdec_set_carveout(void *blob, const char *node, const char *prop_name,
- unsigned int index, const char *name,
- const char **compatibles, unsigned int count,
- const struct fdt_memory *carveout);
+ unsigned int index, const struct fdt_memory *carveout,
+ const char *name, const char **compatibles,
+ unsigned int count);
/**
* Set up the device tree ready for use
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index ba1fefaeef9d..60e537b8d61e 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1561,9 +1561,9 @@ skip_compat:
}
int fdtdec_set_carveout(void *blob, const char *node, const char *prop_name,
- unsigned int index, const char *name,
- const char **compatibles, unsigned int count,
- const struct fdt_memory *carveout)
+ unsigned int index, const struct fdt_memory *carveout,
+ const char *name, const char **compatibles,
+ unsigned int count)
{
uint32_t phandle;
int err, offset, len;
diff --git a/lib/fdtdec_test.c b/li
[PATCH 2/9] fdtdec: Support retrieving the name of a carveout
From: Thierry Reding
When retrieving a given carveout for a device, allow callers to query
the name. This helps differentiating between carveouts when there are
more than one.
This is also useful when copying carveouts to help assign a meaningful
name that cannot always be guessed.
Signed-off-by: Thierry Reding
---
board/nvidia/p2371-2180/p2371-2180.c | 2 +-
board/nvidia/p2771-/p2771-.c | 2 +-
board/nvidia/p3450-/p3450-.c | 2 +-
include/fdtdec.h | 8 +---
lib/fdtdec.c | 12
lib/fdtdec_test.c| 3 ++-
6 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/board/nvidia/p2371-2180/p2371-2180.c
b/board/nvidia/p2371-2180/p2371-2180.c
index 7423a97ad0e3..1f7aa0050cde 100644
--- a/board/nvidia/p2371-2180/p2371-2180.c
+++ b/board/nvidia/p2371-2180/p2371-2180.c
@@ -128,7 +128,7 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
struct fdt_memory fb;
int err;
- err = fdtdec_get_carveout(src, node, "memory-region", 0, );
+ err = fdtdec_get_carveout(src, node, "memory-region", 0, , NULL);
if (err < 0) {
if (err != -FDT_ERR_NOTFOUND)
printf("failed to get carveout for %s: %d\n", node,
diff --git a/board/nvidia/p2771-/p2771-.c
b/board/nvidia/p2771-/p2771-.c
index 508c4d27b7fb..aca86c342680 100644
--- a/board/nvidia/p2771-/p2771-.c
+++ b/board/nvidia/p2771-/p2771-.c
@@ -104,7 +104,7 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
struct fdt_memory fb;
int err;
- err = fdtdec_get_carveout(src, node, "memory-region", 0, );
+ err = fdtdec_get_carveout(src, node, "memory-region", 0, , NULL);
if (err < 0) {
if (err != -FDT_ERR_NOTFOUND)
printf("failed to get carveout for %s: %d\n", node,
diff --git a/board/nvidia/p3450-/p3450-.c
b/board/nvidia/p3450-/p3450-.c
index f4741cfd1e4a..132eb824c675 100644
--- a/board/nvidia/p3450-/p3450-.c
+++ b/board/nvidia/p3450-/p3450-.c
@@ -128,7 +128,7 @@ static int ft_copy_carveout(void *dst, const void *src,
const char *node)
struct fdt_memory fb;
int err;
- err = fdtdec_get_carveout(src, node, "memory-region", 0, );
+ err = fdtdec_get_carveout(src, node, "memory-region", 0, , NULL);
if (err < 0) {
if (err != -FDT_ERR_NOTFOUND)
printf("failed to get carveout for %s: %d\n", node,
diff --git a/include/fdtdec.h b/include/fdtdec.h
index 8ac20c9a64f7..265ee54d41be 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -1071,14 +1071,16 @@ int fdtdec_add_reserved_memory(void *blob, const char
*basename,
*
* @param blob FDT blob
* @param node name of a node
- * @param name name of the property in the given node that contains
+ * @param prop_namename of the property in the given node that contains
* the phandle for the carveout
* @param indexindex of the phandle for which to read the
carveout
* @param carveout return location for the carveout information
+ * @param name return location for the carveout name
* @return 0 on success or a negative error code on failure
*/
-int fdtdec_get_carveout(const void *blob, const char *node, const char *name,
- unsigned int index, struct fdt_memory *carveout);
+int fdtdec_get_carveout(const void *blob, const char *node,
+ const char *prop_name, unsigned int index,
+ struct fdt_memory *carveout, const char **name);
/**
* fdtdec_set_carveout() - sets a carveout region for a given node
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 7f6b6d523232..19b8efb0d302 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1444,8 +1444,9 @@ int fdtdec_add_reserved_memory(void *blob, const char
*basename,
return 0;
}
-int fdtdec_get_carveout(const void *blob, const char *node, const char *name,
- unsigned int index, struct fdt_memory *carveout)
+int fdtdec_get_carveout(const void *blob, const char *node,
+ const char *prop_name, unsigned int index,
+ struct fdt_memory *carveout, const char **name)
{
const fdt32_t *prop;
uint32_t phandle;
@@ -1456,9 +1457,9 @@ int fdtdec_get_carveout(const void *blob, const char
*node, const char *name,
if (offset < 0)
return offset;
- prop = fdt_getprop(blob, offset, name, );
+ prop = fdt_getprop(blob, offset, prop_name, );
if (!prop) {
- debug("failed to get %s for %s\n", name, node);
+ debug("failed to get %s for %s\n", prop_n
[PATCH 1/9] fdtdec: Allow using fdtdec_get_carveout() in loops
From: Thierry Reding
In order make it possible to use fdtdec_get_carveout() in loops, return
FDT_ERR_NOTFOUND when the passed-in index exceeds the number of phandles
present in the given property.
Signed-off-by: Thierry Reding
---
lib/fdtdec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 337c4443b032..7f6b6d523232 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1469,7 +1469,7 @@ int fdtdec_get_carveout(const void *blob, const char
*node, const char *name,
if (len < (sizeof(phandle) * (index + 1))) {
debug("invalid phandle index\n");
- return -FDT_ERR_BADPHANDLE;
+ return -FDT_ERR_NOTFOUND;
}
phandle = fdt32_to_cpu(prop[index]);
--
2.33.0
[PATCH 0/9] ARM: tegra: Support EMC frequency tables on Tegra210
From: Thierry Reding Hi, The first handful of patches are preparatory work to make the fdtdec carveout helpers a bit more flexible and clean them up a little bit while the final 4 patches make use of the improved helpers to copy the EMC frequency tables that can be passed to U-Boot from earlier bootloaders. Thanks, Thierry Thierry Reding (9): fdtdec: Allow using fdtdec_get_carveout() in loops fdtdec: Support retrieving the name of a carveout fdtdec: Support compatible string list for reserved memory fdtdec: Reorder fdtdec_set_carveout() parameters for consistency fdtdec: Support reserved-memory flags ARM: tegra: Support multiple reserved memory regions ARM: tegra: Support EMC frequency tables on Tegra210 ARM: tegra: Refactor DT update helpers ARM: tegra: Copy memory-region-names property arch/arm/cpu/armv8/fsl-layerscape/soc.c | 3 +- arch/arm/include/asm/arch-tegra/board.h | 10 ++ arch/arm/mach-tegra/dt-setup.c | 147 arch/riscv/lib/fdt_fixup.c | 2 +- board/nvidia/p2371-2180/p2371-2180.c| 94 ++- board/nvidia/p2771-/p2771-.c| 98 ++-- board/nvidia/p3450-/p3450-.c| 96 ++-- include/configs/tegra210-common.h | 2 +- include/fdtdec.h| 39 +-- lib/fdtdec.c| 99 ++-- lib/fdtdec_test.c | 7 +- lib/optee/optee.c | 4 +- test/dm/fdtdec.c| 28 +++-- 13 files changed, 321 insertions(+), 308 deletions(-) -- 2.33.0
Re: [PATCH 3/3] ARM: tegra: p2371-2180: add I2C nodes to DT
On Wed, Apr 01, 2020 at 10:35:23PM +0200, Tom Warren wrote:
> -Original Message-
> From: Thierry Reding
> Sent: Wednesday, April 1, 2020 8:20 AM
> To: Tom Warren
> Cc: Peter Robinson ; tomcwarren3...@gmail.com;
> u-boot@lists.denx.de; Stephen Warren ; Jonathan Hunter
> ; Vishruth Jain
> Subject: Re: [PATCH 3/3] ARM: tegra: p2371-2180: add I2C nodes to DT
>
> On Wed, Apr 01, 2020 at 02:03:09AM +0200, Tom Warren wrote:
> > -Original Message-
> > From: Peter Robinson
> > Sent: Tuesday, March 31, 2020 3:54 AM
> > To: tomcwarren3...@gmail.com
> > Cc: u-boot@lists.denx.de; Stephen Warren ; Thierry
> > Reding ; Jonathan Hunter ;
> > Tom Warren ; Vishruth Jain
> > Subject: Re: [PATCH 3/3] ARM: tegra: p2371-2180: add I2C nodes to DT
> >
> > External email: Use caution opening links or attachments
> >
> >
> > > From: Stephen Warren
> > >
> > > This adds to the DT the I2C controllers that connect to the board ID
> > > EEPROM, camera board EEPROM, etc. With this change, you can now
> > > probe all I2C devices on a TX1 board.
> > >
> > > Signed-off-by: Tom Warren
> > > ---
> > > arch/arm/dts/tegra210-p2371-2180.dts | 18 ++
> > > 1 file changed, 18 insertions(+)
> > >
> > > diff --git a/arch/arm/dts/tegra210-p2371-2180.dts
> > > b/arch/arm/dts/tegra210-p2371-2180.dts
> > > index c2f497c..d982b5f 100644
> > > --- a/arch/arm/dts/tegra210-p2371-2180.dts
> > > +++ b/arch/arm/dts/tegra210-p2371-2180.dts
> > > @@ -12,6 +12,9 @@
> > >
> > > aliases {
> > > i2c0 = "/i2c@7000d000";
> > > + i2c2 = "/i2c@7000c400";
> > > + i2c3 = "/i2c@7000c500";
> > > + i2c5 = "/i2c@546c0c00";
> >
> > I don't think this is correct, it doesn't show up in U-Boot with the
> > "i2c bus" command where the others do, looking in the tegra210.dtsi it
> > looks like it should be i2c@546c?
> > [Tom] That I2C address is working in downstream (L4T) TX1 U-Boot. The
> > VI_I2C controller is a little weird, it's normal I2C registers are
> > offset from base by 0xC00. A different driver is needed, but I
> > haven't posted it yet upstream. I should probably drop if from the
> > DTS for now until I post the VI_I2C driver.
>
> I think the problem here is that the upstream U-Boot device tree
> doesn't contain an i2c@546c0c00 node. Instead it has i2c@546c,
> which we also have in the upstream kernel. My recollection is that
> that's also the address listed in the Tegra210 system address map of
> the TRM and there are some registers before the regular I2C interface
> at offset 0xc00.
>
> I've been carrying a patch against the upstream Linux I2C controller
> driver to special-case the VI/I2C to always add that 0xc00 offset when
> accessing registers, which allows us to reuse the existing driver and
> at the same time keeps all registers mapped so we can also access the
> VI/I2C specific registers.
>
> My recollection is that the U-Boot driver is fairly similar to the
> Linux driver, so I suspect something similar could be done.
>
> Thierry
> [Tom] Thanks, Thierry. That's my recollection, too, about the VI_I2C
> 0xC00 offset. I'll take a look at what we have in L4T U-Boot for T210
> and address it in a set of patches for upstream soon. That I2C
> controller isn't used for anything on any Jetson board except on TX1,
> I believe, where it allows U-Boot to talk to the camera add-in board
> to read the board ID. But we've moved all that out to CBoot (board ID
> EEPROM reading), so there isn't a pressing need for it in U-Boot
> anymore, IIRC.
We've had internal discussions about potentially using the ID EEPROMs to
dynamically modify the kernel DTB at runtime (possibly from U-Boot) in
order to support things like different add-in boards.
For example, we currently enable the DSI output in Linux for Jetson TX1.
However, the Jetson TX1 developer kits don't actually ship with that DSI
display (I don't think they even come with the display add-in card), so
this can lead to confusion. The idea was to have U-Boot probe ID EEPROMs
from several sources to determine what to add to the device tree, so
that the kernel DT *source* would only contain the standard developer
kit hardware, but U-Boot (or something else perhaps) could add in extra
nodes for a display module, or camera add-in board, etc.
Note that this is just a vague idea at this time and nothing concrete
has been done to implement this, yet.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH 3/3] ARM: tegra: p2371-2180: add I2C nodes to DT
On Wed, Apr 01, 2020 at 02:03:09AM +0200, Tom Warren wrote:
> -Original Message-
> From: Peter Robinson
> Sent: Tuesday, March 31, 2020 3:54 AM
> To: tomcwarren3...@gmail.com
> Cc: u-boot@lists.denx.de; Stephen Warren ; Thierry Reding
> ; Jonathan Hunter ; Tom Warren
> ; Vishruth Jain
> Subject: Re: [PATCH 3/3] ARM: tegra: p2371-2180: add I2C nodes to DT
>
> External email: Use caution opening links or attachments
>
>
> > From: Stephen Warren
> >
> > This adds to the DT the I2C controllers that connect to the board ID
> > EEPROM, camera board EEPROM, etc. With this change, you can now probe
> > all I2C devices on a TX1 board.
> >
> > Signed-off-by: Tom Warren
> > ---
> > arch/arm/dts/tegra210-p2371-2180.dts | 18 ++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/arch/arm/dts/tegra210-p2371-2180.dts
> > b/arch/arm/dts/tegra210-p2371-2180.dts
> > index c2f497c..d982b5f 100644
> > --- a/arch/arm/dts/tegra210-p2371-2180.dts
> > +++ b/arch/arm/dts/tegra210-p2371-2180.dts
> > @@ -12,6 +12,9 @@
> >
> > aliases {
> > i2c0 = "/i2c@7000d000";
> > + i2c2 = "/i2c@7000c400";
> > + i2c3 = "/i2c@7000c500";
> > + i2c5 = "/i2c@546c0c00";
>
> I don't think this is correct, it doesn't show up in U-Boot with the
> "i2c bus" command where the others do, looking in the tegra210.dtsi it
> looks like it should be i2c@546c?
> [Tom] That I2C address is working in downstream (L4T) TX1 U-Boot. The
> VI_I2C controller is a little weird, it's normal I2C registers are
> offset from base by 0xC00. A different driver is needed, but I
> haven't posted it yet upstream. I should probably drop if from the
> DTS for now until I post the VI_I2C driver.
I think the problem here is that the upstream U-Boot device tree doesn't
contain an i2c@546c0c00 node. Instead it has i2c@546c, which we also
have in the upstream kernel. My recollection is that that's also the
address listed in the Tegra210 system address map of the TRM and there
are some registers before the regular I2C interface at offset 0xc00.
I've been carrying a patch against the upstream Linux I2C controller
driver to special-case the VI/I2C to always add that 0xc00 offset when
accessing registers, which allows us to reuse the existing driver and at
the same time keeps all registers mapped so we can also access the
VI/I2C specific registers.
My recollection is that the U-Boot driver is fairly similar to the Linux
driver, so I suspect something similar could be done.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH 1/2] net: rt8169: WAR for DHCP not getting IP after kernel boot/reboot
On Tue, Mar 17, 2020 at 01:07:15PM -0700, twar...@nvidia.com wrote: > From: Tom Warren > > This is a WAR for DHCP failure after rebooting from the L4T kernel. The > r8169.c kernel driver is setting bit 19 of the rt816x HW register 0xF0, > which goes by FuncEvent and MISC in various driver source/datasheets. > That bit is called RxDv_Gated_En in the r8169.c kernel driver. Clear it > here at the end of probe to ensure that U-Boot can get an IP assigned > via DHCP. > > Signed-off-by: Tom Warren > --- > drivers/net/rtl8169.c | 16 > 1 file changed, 16 insertions(+) Is this still needed? In my old p3450 branch that I worked on to get Porg up and running I don't have this patch. It's also not in my local development tree that I typically use to boot Tegra186 and earlier boards. That branch works fine on the Jetson Nano, so I don't think this is needed anymore. I vaguely recall that I determined that this was fixed some other way, but unfortunately I don't remember the exact details. Thierry signature.asc Description: PGP signature
[U-Boot] [PATCH] net: rtl8169: Support RTL-8168c/8111c
From: Thierry Reding
This version of the RTL-8168 chip can be found on some add-in cards sold
by CSL-Computer GmbH & Co. KG. The chip isn't special in any way, but it
needs to have the ChipCmd register programmed after the DMA descriptors
have been set up, so make sure that happens by adding an entry to the
chip information table.
Signed-off-by: Thierry Reding
---
drivers/net/rtl8169.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
index 521e5909a256..d887d00928f6 100644
--- a/drivers/net/rtl8169.c
+++ b/drivers/net/rtl8169.c
@@ -252,6 +252,7 @@ static struct {
{"RTL-8169sc/8110sc", 0x18, 0xff7e1880,},
{"RTL-8168b/8111sb",0x30, 0xff7e1880,},
{"RTL-8168b/8111sb",0x38, 0xff7e1880,},
+ {"RTL-8168c/8111c", 0x3c, 0xff7e1880,},
{"RTL-8168d/8111d", 0x28, 0xff7e1880,},
{"RTL-8168evl/8111evl", 0x2e, 0xff7e1880,},
{"RTL-8168/8111g", 0x4c, 0xff7e1880,},
--
2.23.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] rtl8169: use dm_pci_map_bar
On Fri, Jun 14, 2019 at 01:16:32AM +0200, Patrick Wildt wrote:
> On Thu, Jun 13, 2019 at 06:43:25PM +0200, Thierry Reding wrote:
> > On Thu, Jun 13, 2019 at 03:16:10PM +0800, Bin Meng wrote:
> > > Hi Stefan,
> > >
> > > On Thu, Jun 13, 2019 at 1:40 PM Stefan Roese wrote:
> > > >
> > > > Added Bin, Joe and Thierry to Cc
> > > >
> > > > On 11.06.19 13:15, Patrick Wildt wrote:
> > > > > Hi,
> > > > >
> > > > > I have an rtl8169 on a macchiatobin and that card has a 64-bit
> > > > > memory address. The current code only reads a single word, which
> > > > > means it can only support a 32-bit address. By using dm_pci_map_bar
> > > > > we don't need to manually parse the register, we can just have it do
> > > > > its job.
> > > > >
> > > > > I'm not sure though if this works for all devices since the previous
> > > > > version had an explicit check for the device.
> > > > >
> > > > > Patrick
> > > > >
> > > > > Signed-off-by: Patrick Wildt
> > > > >
> > > > > diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
> > > > > index 521e5909a2..f1d2ade253 100644
> > > > > --- a/drivers/net/rtl8169.c
> > > > > +++ b/drivers/net/rtl8169.c
> > > > > @@ -1182,22 +1182,11 @@ static int rtl8169_eth_probe(struct udevice
> > > > > *dev)
> > > > > struct pci_child_platdata *pplat = dev_get_parent_platdata(dev);
> > > > > struct rtl8169_private *priv = dev_get_priv(dev);
> > > > > struct eth_pdata *plat = dev_get_platdata(dev);
> > > > > - u32 iobase;
> > > > > - int region;
> > > > > int ret;
> > > > >
> > > > > - debug("rtl8169: REALTEK RTL8169 @0x%x\n", iobase);
> > > > > - switch (pplat->device) {
> > > > > - case 0x8168:
> > > > > - region = 2;
> > > > > - break;
> > > > > - default:
> > > > > - region = 1;
> > > > > - break;
> > > > > - }
> > > > > - dm_pci_read_config32(dev, PCI_BASE_ADDRESS_0 + region * 4,
> > > > > );
> > > > > - iobase &= ~0xf;
> > > > > - priv->iobase = (int)dm_pci_mem_to_phys(dev, iobase);
> > > > > + priv->iobase = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_2,
> > > > > + PCI_REGION_MEM);
> > > > > + printf("rtl8169: REALTEK RTL8169 @0x%x\n", priv->iobase);
> > > > >
> > > > > ret = rtl_init(priv->iobase, dev->name, plat->enetaddr);
> > > > > if (ret < 0) {
> > > >
> > > > Bin, Joe, Thierry,
> > > >
> > > > do you have any comments on this patch? Moving unconditionally to one
> > > > BAR instead of BAR1/2 depending on the chip version seems a bit
> > > > "brave".
> > >
> > > Agreed that blinding setting one BAR for the iobase is not a good idea.
> >
> > Agreed. I don't know whether it's actually required to differentiate
> > based on version, but I suppose the code is like that for a reason, so
> > better keep that.
> >
> > Also, looking at dm_pci_map_bar() it doesn't look like that does
> > anything different than the existing code. It merely reads a single
> > 32-bit register, so it doesn't properly deal with 64-bit BARs either.
> >
> > I suppose that could be fixed in dm_pci_map_bar(), and then the fix
> > would automatically propagate to all users of that, which is good. But I
> > don't think it currently works correctly.
> >
> > Thierry
>
> Oh, I'm very sorry to have wasted your time. Yes, I agree, the
> differentiation must come back.
>
> To be honest it was about half a year ago, so I think I forgot the
> actual issue. Maybe dm_pci_mem_to_phys() returned 0 for whatever
> reason, but I don't remember. The address is 32-bit, 0xf600,
> so it definitely wasn't the 64-bit think I rambled about. Sorry.
>
> I will try to find out what was wrong and report back.
>
> By the way, the debug() line in the current code prints iobase which at
> that time isn't even initialized.
Yeah, that's definitely a bug. I think that warrants a separate patch.
Mind sending out a patch with just that change?
Thierry
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] rtl8169: use dm_pci_map_bar
On Thu, Jun 13, 2019 at 03:16:10PM +0800, Bin Meng wrote:
> Hi Stefan,
>
> On Thu, Jun 13, 2019 at 1:40 PM Stefan Roese wrote:
> >
> > Added Bin, Joe and Thierry to Cc
> >
> > On 11.06.19 13:15, Patrick Wildt wrote:
> > > Hi,
> > >
> > > I have an rtl8169 on a macchiatobin and that card has a 64-bit
> > > memory address. The current code only reads a single word, which
> > > means it can only support a 32-bit address. By using dm_pci_map_bar
> > > we don't need to manually parse the register, we can just have it do
> > > its job.
> > >
> > > I'm not sure though if this works for all devices since the previous
> > > version had an explicit check for the device.
> > >
> > > Patrick
> > >
> > > Signed-off-by: Patrick Wildt
> > >
> > > diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
> > > index 521e5909a2..f1d2ade253 100644
> > > --- a/drivers/net/rtl8169.c
> > > +++ b/drivers/net/rtl8169.c
> > > @@ -1182,22 +1182,11 @@ static int rtl8169_eth_probe(struct udevice *dev)
> > > struct pci_child_platdata *pplat = dev_get_parent_platdata(dev);
> > > struct rtl8169_private *priv = dev_get_priv(dev);
> > > struct eth_pdata *plat = dev_get_platdata(dev);
> > > - u32 iobase;
> > > - int region;
> > > int ret;
> > >
> > > - debug("rtl8169: REALTEK RTL8169 @0x%x\n", iobase);
> > > - switch (pplat->device) {
> > > - case 0x8168:
> > > - region = 2;
> > > - break;
> > > - default:
> > > - region = 1;
> > > - break;
> > > - }
> > > - dm_pci_read_config32(dev, PCI_BASE_ADDRESS_0 + region * 4, );
> > > - iobase &= ~0xf;
> > > - priv->iobase = (int)dm_pci_mem_to_phys(dev, iobase);
> > > + priv->iobase = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_2,
> > > + PCI_REGION_MEM);
> > > + printf("rtl8169: REALTEK RTL8169 @0x%x\n", priv->iobase);
> > >
> > > ret = rtl_init(priv->iobase, dev->name, plat->enetaddr);
> > > if (ret < 0) {
> >
> > Bin, Joe, Thierry,
> >
> > do you have any comments on this patch? Moving unconditionally to one
> > BAR instead of BAR1/2 depending on the chip version seems a bit
> > "brave".
>
> Agreed that blinding setting one BAR for the iobase is not a good idea.
Agreed. I don't know whether it's actually required to differentiate
based on version, but I suppose the code is like that for a reason, so
better keep that.
Also, looking at dm_pci_map_bar() it doesn't look like that does
anything different than the existing code. It merely reads a single
32-bit register, so it doesn't properly deal with 64-bit BARs either.
I suppose that could be fixed in dm_pci_map_bar(), and then the fix
would automatically propagate to all users of that, which is good. But I
don't think it currently works correctly.
Thierry
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/2] net: eth-uclass: Support device tree MAC addresses
On Mon, May 20, 2019 at 05:59:57PM +0200, Thierry Reding wrote: > From: Thierry Reding > > Add the standard Ethernet device tree bindings (imported from v5.0 of > the Linux kernel) and implement support for reading the MAC address for > Ethernet devices in the Ethernet uclass. If the "mac-address" property > exists, the MAC address will be parsed from that. If that property does > not exist, the "local-mac-address" property will be tried as fallback. > > MAC addresses from device tree take precedence over the ones stored in > a network interface card's ROM. > > Acked-by: Joe Hershberger > Reviewed-by: Grygorii Strashko > Signed-off-by: Thierry Reding > --- > Changes in v4: > - do not clear MAC address on failure to avoid overwriting existing MAC > - fix build on xtensa (missing CONFIG_OF_CONTROL) > > Changes in v3: > - add additional check to make sure the MAC address read from device > tree is a valid MAC address > > Changes in v2: > - use dev_read_u8_array_ptr() > --- > .../devicetree/bindings/net/ethernet.txt | 66 +++ > net/eth-uclass.c | 30 - > 2 files changed, 93 insertions(+), 3 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/ethernet.txt Hi Joe, did you have a chance to look at this updated version? I've manually verified that the qemu-x86_64 test failure is no longer there and the build failure for xtensa is also gone. A fairly largish series for Tom's Tegra tree is currently blocked on this because without these two patches it will cause a test failure. Thierry > diff --git a/Documentation/devicetree/bindings/net/ethernet.txt > b/Documentation/devicetree/bindings/net/ethernet.txt > new file mode 100644 > index ..cfc376bc977a > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/ethernet.txt > @@ -0,0 +1,66 @@ > +The following properties are common to the Ethernet controllers: > + > +NOTE: All 'phy*' properties documented below are Ethernet specific. For the > +generic PHY 'phys' property, see > +Documentation/devicetree/bindings/phy/phy-bindings.txt. > + > +- local-mac-address: array of 6 bytes, specifies the MAC address that was > + assigned to the network device; > +- mac-address: array of 6 bytes, specifies the MAC address that was last > used by > + the boot program; should be used in cases where the MAC address assigned to > + the device by the boot program is different from the "local-mac-address" > + property; > +- nvmem-cells: phandle, reference to an nvmem node for the MAC address; > +- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used; > +- max-speed: number, specifies maximum speed in Mbit/s supported by the > device; > +- max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather > than > + the maximum frame size (there's contradiction in the Devicetree > + Specification). > +- phy-mode: string, operation mode of the PHY interface. This is now a > de-facto > + standard property; supported values are: > + * "internal" > + * "mii" > + * "gmii" > + * "sgmii" > + * "qsgmii" > + * "tbi" > + * "rev-mii" > + * "rmii" > + * "rgmii" (RX and TX delays are added by the MAC when required) > + * "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, the > + MAC should not add the RX or TX delays in this case) > + * "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, the MAC > + should not add an RX delay in this case) > + * "rgmii-txid" (RGMII with internal TX delay provided by the PHY, the MAC > + should not add an TX delay in this case) > + * "rtbi" > + * "smii" > + * "xgmii" > + * "trgmii" > + * "2000base-x", > + * "2500base-x", > + * "rxaui" > + * "xaui" > + * "10gbase-kr" (10GBASE-KR, XFI, SFI) > +- phy-connection-type: the same as "phy-mode" property but described in the > + Devicetree Specification; > +- phy-handle: phandle, specifies a reference to a node representing a PHY > + device; this property is described in the Devicetree Specification and so > + preferred; > +- phy: the same as "phy-handle" property, not recommended for new bindings. > +- phy-device: the same as "phy-handle" property, not recommended for new > + bindings. > +- rx-fifo-depth: the size of the controller's receive fifo in bytes. This > + is used for components that can have configurable receive fifo sizes, > + and is useful
Re: [U-Boot] [PATCH 1/2] fdtdec: test: Fix memory leak
On Mon, May 20, 2019 at 02:51:08PM -0600, Simon Glass wrote:
> Hi Thierry,
>
> On Mon, 20 May 2019 at 10:05, Thierry Reding wrote:
> >
> > From: Thierry Reding
> >
> > Free the memory allocated to store the test FDT upon test completion to
> > avoid leaking the memory. We don't bother cleaning up on test failure
> > since the code is broken in that case and should be fixed, in which case
> > the leak would also go away.
> >
> > Reported-by: Tom Rini
> > Suggested-by: Heinrich Schuchardt
> > Signed-off-by: Thierry Reding
> > ---
> > lib/fdtdec_test.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/fdtdec_test.c b/lib/fdtdec_test.c
> > index f6defe16c5a6..54efcc3d46ac 100644
> > --- a/lib/fdtdec_test.c
> > +++ b/lib/fdtdec_test.c
> > @@ -138,6 +138,7 @@ static int run_test(const char *aliases, const char
> > *nodes, const char *expect)
> > }
> >
> > printf("pass\n");
> > + free(blob);
>
> Strictly speaking, CHECKVAL() can cause a function return in the case
> of an error.
>
> So a better solution might be to put the code after the malloc() into
> a separate function.
When Heinrich suggested this fix he brought up the same issue, but
concluded, and I agree with him, that it wasn't worth addressing the
CHECKVAL case because if CHECKVAL failed, our code was buggy and would
need fixing, at which point the leak would go away along with the bug.
Do you feel strongly about reworking this so it doesn't leak in the
error case either?
Thierry
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 2/2] fdtdec: Remove fdt_{addr,size}_unpack()
From: Thierry Reding
U-Boot already defines the {upper,lower}_32_bits() macros that have the
same purpose. Use the existing macros instead of defining new APIs.
Signed-off-by: Thierry Reding
---
include/fdtdec.h | 24
lib/fdtdec.c | 8 ++--
lib/fdtdec_test.c | 8 ++--
3 files changed, 12 insertions(+), 28 deletions(-)
diff --git a/include/fdtdec.h b/include/fdtdec.h
index 110aa6ab6dea..fa8e34f6f960 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -24,30 +24,6 @@
typedef phys_addr_t fdt_addr_t;
typedef phys_size_t fdt_size_t;
-static inline fdt32_t fdt_addr_unpack(fdt_addr_t addr, fdt32_t *upper)
-{
- if (upper)
-#ifdef CONFIG_PHYS_64BIT
- *upper = addr >> 32;
-#else
- *upper = 0;
-#endif
-
- return addr;
-}
-
-static inline fdt32_t fdt_size_unpack(fdt_size_t size, fdt32_t *upper)
-{
- if (upper)
-#ifdef CONFIG_PHYS_64BIT
- *upper = size >> 32;
-#else
- *upper = 0;
-#endif
-
- return size;
-}
-
#ifdef CONFIG_PHYS_64BIT
#define FDT_ADDR_T_NONE (-1U)
#define fdt_addr_to_cpu(reg) be64_to_cpu(reg)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index fea44a9a8c65..d0ba88897335 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1300,6 +1300,7 @@ int fdtdec_add_reserved_memory(void *blob, const char
*basename,
fdt32_t cells[4] = {}, *ptr = cells;
uint32_t upper, lower, phandle;
int parent, node, na, ns, err;
+ fdt_size_t size;
char name[64];
/* create an empty /reserved-memory node if one doesn't exist */
@@ -1340,7 +1341,8 @@ int fdtdec_add_reserved_memory(void *blob, const char
*basename,
* Unpack the start address and generate the name of the new node
* base on the basename and the unit-address.
*/
- lower = fdt_addr_unpack(carveout->start, );
+ upper = upper_32_bits(carveout->start);
+ lower = lower_32_bits(carveout->start);
if (na > 1 && upper > 0)
snprintf(name, sizeof(name), "%s@%x,%x", basename, upper,
@@ -1374,7 +1376,9 @@ int fdtdec_add_reserved_memory(void *blob, const char
*basename,
*ptr++ = cpu_to_fdt32(lower);
/* store one or two size cells */
- lower = fdt_size_unpack(carveout->end - carveout->start + 1, );
+ size = carveout->end - carveout->start + 1;
+ upper = upper_32_bits(size);
+ lower = lower_32_bits(size);
if (ns > 1)
*ptr++ = cpu_to_fdt32(upper);
diff --git a/lib/fdtdec_test.c b/lib/fdtdec_test.c
index 54efcc3d46ac..e8bfd1fb1ec3 100644
--- a/lib/fdtdec_test.c
+++ b/lib/fdtdec_test.c
@@ -156,11 +156,13 @@ static int make_fdt_carveout_device(void *fdt, uint32_t
na, uint32_t ns)
};
fdt32_t cells[4], *ptr = cells;
uint32_t upper, lower;
+ fdt_size_t size;
char name[32];
int offset;
/* store one or two address cells */
- lower = fdt_addr_unpack(carveout.start, );
+ upper = upper_32_bits(carveout.start);
+ lower = lower_32_bits(carveout.start);
if (na > 1 && upper > 0)
snprintf(name, sizeof(name), "%s@%x,%x", basename, upper,
@@ -174,7 +176,9 @@ static int make_fdt_carveout_device(void *fdt, uint32_t na,
uint32_t ns)
*ptr++ = cpu_to_fdt32(lower);
/* store one or two size cells */
- lower = fdt_size_unpack(carveout.end - carveout.start + 1, );
+ size = carveout.end - carveout.start + 1;
+ upper = upper_32_bits(size);
+ lower = lower_32_bits(size);
if (ns > 1)
*ptr++ = cpu_to_fdt32(upper);
--
2.21.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 1/2] fdtdec: test: Fix memory leak
From: Thierry Reding
Free the memory allocated to store the test FDT upon test completion to
avoid leaking the memory. We don't bother cleaning up on test failure
since the code is broken in that case and should be fixed, in which case
the leak would also go away.
Reported-by: Tom Rini
Suggested-by: Heinrich Schuchardt
Signed-off-by: Thierry Reding
---
lib/fdtdec_test.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/fdtdec_test.c b/lib/fdtdec_test.c
index f6defe16c5a6..54efcc3d46ac 100644
--- a/lib/fdtdec_test.c
+++ b/lib/fdtdec_test.c
@@ -138,6 +138,7 @@ static int run_test(const char *aliases, const char *nodes,
const char *expect)
}
printf("pass\n");
+ free(blob);
return 0;
}
@@ -288,6 +289,7 @@ static int check_carveout(void)
CHECKVAL(make_fdt_carveout(fdt, FDT_SIZE, 2, 2), 0);
CHECKOK(check_fdt_carveout(fdt, 2, 2));
+ free(fdt);
return 0;
}
--
2.21.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 2/2] net: eth-uclass: Support device tree MAC addresses
From: Thierry Reding Add the standard Ethernet device tree bindings (imported from v5.0 of the Linux kernel) and implement support for reading the MAC address for Ethernet devices in the Ethernet uclass. If the "mac-address" property exists, the MAC address will be parsed from that. If that property does not exist, the "local-mac-address" property will be tried as fallback. MAC addresses from device tree take precedence over the ones stored in a network interface card's ROM. Acked-by: Joe Hershberger Reviewed-by: Grygorii Strashko Signed-off-by: Thierry Reding --- Changes in v4: - do not clear MAC address on failure to avoid overwriting existing MAC - fix build on xtensa (missing CONFIG_OF_CONTROL) Changes in v3: - add additional check to make sure the MAC address read from device tree is a valid MAC address Changes in v2: - use dev_read_u8_array_ptr() --- .../devicetree/bindings/net/ethernet.txt | 66 +++ net/eth-uclass.c | 30 - 2 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/ethernet.txt diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt new file mode 100644 index ..cfc376bc977a --- /dev/null +++ b/Documentation/devicetree/bindings/net/ethernet.txt @@ -0,0 +1,66 @@ +The following properties are common to the Ethernet controllers: + +NOTE: All 'phy*' properties documented below are Ethernet specific. For the +generic PHY 'phys' property, see +Documentation/devicetree/bindings/phy/phy-bindings.txt. + +- local-mac-address: array of 6 bytes, specifies the MAC address that was + assigned to the network device; +- mac-address: array of 6 bytes, specifies the MAC address that was last used by + the boot program; should be used in cases where the MAC address assigned to + the device by the boot program is different from the "local-mac-address" + property; +- nvmem-cells: phandle, reference to an nvmem node for the MAC address; +- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used; +- max-speed: number, specifies maximum speed in Mbit/s supported by the device; +- max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather than + the maximum frame size (there's contradiction in the Devicetree + Specification). +- phy-mode: string, operation mode of the PHY interface. This is now a de-facto + standard property; supported values are: + * "internal" + * "mii" + * "gmii" + * "sgmii" + * "qsgmii" + * "tbi" + * "rev-mii" + * "rmii" + * "rgmii" (RX and TX delays are added by the MAC when required) + * "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, the + MAC should not add the RX or TX delays in this case) + * "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, the MAC + should not add an RX delay in this case) + * "rgmii-txid" (RGMII with internal TX delay provided by the PHY, the MAC + should not add an TX delay in this case) + * "rtbi" + * "smii" + * "xgmii" + * "trgmii" + * "2000base-x", + * "2500base-x", + * "rxaui" + * "xaui" + * "10gbase-kr" (10GBASE-KR, XFI, SFI) +- phy-connection-type: the same as "phy-mode" property but described in the + Devicetree Specification; +- phy-handle: phandle, specifies a reference to a node representing a PHY + device; this property is described in the Devicetree Specification and so + preferred; +- phy: the same as "phy-handle" property, not recommended for new bindings. +- phy-device: the same as "phy-handle" property, not recommended for new + bindings. +- rx-fifo-depth: the size of the controller's receive fifo in bytes. This + is used for components that can have configurable receive fifo sizes, + and is useful for determining certain configuration settings such as + flow control thresholds. +- tx-fifo-depth: the size of the controller's transmit fifo in bytes. This + is used for components that can have configurable fifo sizes. +- managed: string, specifies the PHY management type. Supported values are: + "auto", "in-band-status". "auto" is the default, it usess MDIO for + management if fixed-link is not specified. + +Child nodes of the Ethernet controller are typically the individual PHY devices +connected via the MDIO bus (sometimes the MDIO bus controller is separate). +They are described in the phy.txt file in this same directory. +For non-MDIO PHY management see fixed-link.txt. diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 4225aabf1fa1..031d55862583 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -455,6 +455,26 @@
[U-Boot] [PATCH v4 1/2] net: eth-uclass: Write MAC address to hardware after probe
From: Thierry Reding In order for the device to use the proper MAC address, which can have been configured in the environment prior to the device being registered, ensure that the MAC address is written after the device has been probed. For devices that are registered before the network stack is initialized, this is already done during eth_initialize(). If the Ethernet device is on a bus that is not initialized on early boot, such as PCI, the device is not available at the time eth_initialize() is called, so we need the MAC address programming to also happen after probe. Acked-by: Joe Hershberger Signed-off-by: Thierry Reding --- net/eth-uclass.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 2ef20df19203..4225aabf1fa1 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -524,6 +524,8 @@ static int eth_post_probe(struct udevice *dev) #endif } + eth_write_hwaddr(dev); + return 0; } -- 2.21.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 45/50] Revert "pci: Scale MAX_PCI_REGIONS based on CONFIG_NR_DRAM_BANKS"
On Tue, May 07, 2019 at 09:04:16PM -0600, Simon Glass wrote: > Hi Bin, > > On Tue, 7 May 2019 at 03:28, Bin Meng wrote: > > > > Hi Simon, Thierry, > > > > On Fri, May 3, 2019 at 12:22 AM Simon Glass wrote: > > > > > > Hi Thierry, > > > > > > On Thu, 2 May 2019 at 03:25, Thierry Reding wrote: > > > > > > > > On Thu, May 02, 2019 at 12:09:49AM +0800, Bin Meng wrote: > > > > > +Thierry > > > > > > > > > > On Fri, Apr 26, 2019 at 12:00 PM Simon Glass > > > > > wrote: > > > > > > > > > > > > This reverts commit aec4298ccb337106fd0115b91d846a022fdf301d. > > > > > > > > > > > > Unfortunately this has a dramatic impact on the pre-relocation > > > > > > memory > > > > > > used on x86 platforms (increasing it by 2KB) since it increases the > > > > > > overhead for each PCI device from 220 bytes to 412 bytes. > > > > > > > > > > > > The offending line is in UCLASS_DRIVER(pci): > > > > > > > > > > > > .per_device_auto_alloc_size = sizeof(struct pci_controller), > > > > > > > > > > > > This means that all PCI devices have the controller struct > > > > > > associated > > > > > > with them. The solution is to move the regions[] member out of the > > > > > > array, > > > > > > makes its size dynamic, or split UCLASS_PCI into controllers and > > > > > > non-controllers, as the comment suggests. > > > > > > > > > > > > For now, revert the commit to get things running again. > > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > --- > > > > > > > > > > > > Changes in v2: None > > > > > > > > > > > > include/pci.h | 6 +- > > > > > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > > > > > > > > > > > > > Reviewed-by: Bin Meng > > > > > > > > Ugh... so we're trading one regression for another? Can we not live with > > > > the 2 KiB increase on x86 until this has been properly fixed? Currently > > > > this will cause Jetson TX2 to crash if it starts using PCI. > > > > > > Unfortunately this breaks several boards since we are out of memory. > > > > > > I think this needs a better solution to reduce the memory usage down > > > to sensible levels. This is something I should have considered when > > > implementing the PCI uclass, but unfortunately I did not. > > > > > > > Could you please suggest whether I should apply this revert patch for now? > > I suggest a temporary revert since this breaks some x86 boards. > > I think the real fix is to reduce the memory used by PCI devices. > Thierry, do you have time to look at this? Not right away. I may get around to this within a couple of weeks maybe. Thierry signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] fdtdec: Remove fdt_{addr,size}_unpack()
On Mon, May 06, 2019 at 09:52:02PM -0600, Simon Glass wrote:
> Hi Thierry,
>
> On Fri, 26 Apr 2019 at 06:01, Thierry Reding wrote:
> >
> > On Mon, Apr 15, 2019 at 10:08:21AM +0200, Thierry Reding wrote:
> > > From: Thierry Reding
> > >
> > > U-Boot already defines the {upper,lower}_32_bits() macros that have the
> > > same purpose. Use the existing macros instead of defining new APIs.
> > >
> > > Signed-off-by: Thierry Reding
> > > ---
> > > include/fdtdec.h | 24
> > > lib/fdtdec.c | 8 ++--
> > > lib/fdtdec_test.c | 8 ++--
> > > 3 files changed, 12 insertions(+), 28 deletions(-)
> >
> > Hi Simon,
> >
> > you picked up patch 1/2 of this set. Did you have any comments on v2, or
> > did you overlook it?
>
> I marked it as superseded.
>
> http://patchwork.ozlabs.org/patch/1085480/
>
> Is that incorrect?
I think I messed up my earlier email. What I meant to say was whether
you had any comments on patch 2/2. There's no v2 of this patch and it
hasn't been applied. It's also not superseeded by anything. So I think
this should still be applied.
Thierry
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] fdtdec: Use fdt_setprop_u32() for fdtdec_set_phandle()
On Mon, May 06, 2019 at 09:52:00PM -0600, Simon Glass wrote: > Hi Thierry, > > On Thu, 25 Apr 2019 at 07:25, Thierry Reding wrote: > > > > On Mon, Apr 15, 2019 at 10:08:20AM +0200, Thierry Reding wrote: > > > From: Thierry Reding > > > > > > The fdt_setprop_u32() function does everything that we need, so we > > > really only use the function as a convenience wrapper, in which case it > > > can simply be a static inline function. > > > > > > Signed-off-by: Thierry Reding > > > --- > > > include/fdtdec.h | 5 - > > > lib/fdtdec.c | 7 --- > > > 2 files changed, 4 insertions(+), 8 deletions(-) > > > > Hi Simon, > > > > gentle reminder on these. These are the two follow-up patches that you > > had suggested I make on top of the other fdtdec series that you applied > > a couple of weeks ago. > > I think this is applied now? Yes, I can see this in origin/master now. Thierry signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 45/50] Revert "pci: Scale MAX_PCI_REGIONS based on CONFIG_NR_DRAM_BANKS"
On Thu, May 02, 2019 at 12:09:49AM +0800, Bin Meng wrote: > +Thierry > > On Fri, Apr 26, 2019 at 12:00 PM Simon Glass wrote: > > > > This reverts commit aec4298ccb337106fd0115b91d846a022fdf301d. > > > > Unfortunately this has a dramatic impact on the pre-relocation memory > > used on x86 platforms (increasing it by 2KB) since it increases the > > overhead for each PCI device from 220 bytes to 412 bytes. > > > > The offending line is in UCLASS_DRIVER(pci): > > > > .per_device_auto_alloc_size = sizeof(struct pci_controller), > > > > This means that all PCI devices have the controller struct associated > > with them. The solution is to move the regions[] member out of the array, > > makes its size dynamic, or split UCLASS_PCI into controllers and > > non-controllers, as the comment suggests. > > > > For now, revert the commit to get things running again. > > > > Signed-off-by: Simon Glass > > --- > > > > Changes in v2: None > > > > include/pci.h | 6 +- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > Reviewed-by: Bin Meng Ugh... so we're trading one regression for another? Can we not live with the 2 KiB increase on x86 until this has been properly fixed? Currently this will cause Jetson TX2 to crash if it starts using PCI. Thierry signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] fdtdec: Remove fdt_{addr,size}_unpack()
On Mon, Apr 15, 2019 at 10:08:21AM +0200, Thierry Reding wrote:
> From: Thierry Reding
>
> U-Boot already defines the {upper,lower}_32_bits() macros that have the
> same purpose. Use the existing macros instead of defining new APIs.
>
> Signed-off-by: Thierry Reding
> ---
> include/fdtdec.h | 24
> lib/fdtdec.c | 8 ++--
> lib/fdtdec_test.c | 8 ++--
> 3 files changed, 12 insertions(+), 28 deletions(-)
Hi Simon,
you picked up patch 1/2 of this set. Did you have any comments on v2, or
did you overlook it?
Thierry
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index 110aa6ab6dea..fa8e34f6f960 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -24,30 +24,6 @@
> typedef phys_addr_t fdt_addr_t;
> typedef phys_size_t fdt_size_t;
>
> -static inline fdt32_t fdt_addr_unpack(fdt_addr_t addr, fdt32_t *upper)
> -{
> - if (upper)
> -#ifdef CONFIG_PHYS_64BIT
> - *upper = addr >> 32;
> -#else
> - *upper = 0;
> -#endif
> -
> - return addr;
> -}
> -
> -static inline fdt32_t fdt_size_unpack(fdt_size_t size, fdt32_t *upper)
> -{
> - if (upper)
> -#ifdef CONFIG_PHYS_64BIT
> - *upper = size >> 32;
> -#else
> - *upper = 0;
> -#endif
> -
> - return size;
> -}
> -
> #ifdef CONFIG_PHYS_64BIT
> #define FDT_ADDR_T_NONE (-1U)
> #define fdt_addr_to_cpu(reg) be64_to_cpu(reg)
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index fea44a9a8c65..d0ba88897335 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -1300,6 +1300,7 @@ int fdtdec_add_reserved_memory(void *blob, const char
> *basename,
> fdt32_t cells[4] = {}, *ptr = cells;
> uint32_t upper, lower, phandle;
> int parent, node, na, ns, err;
> + fdt_size_t size;
> char name[64];
>
> /* create an empty /reserved-memory node if one doesn't exist */
> @@ -1340,7 +1341,8 @@ int fdtdec_add_reserved_memory(void *blob, const char
> *basename,
>* Unpack the start address and generate the name of the new node
>* base on the basename and the unit-address.
>*/
> - lower = fdt_addr_unpack(carveout->start, );
> + upper = upper_32_bits(carveout->start);
> + lower = lower_32_bits(carveout->start);
>
> if (na > 1 && upper > 0)
> snprintf(name, sizeof(name), "%s@%x,%x", basename, upper,
> @@ -1374,7 +1376,9 @@ int fdtdec_add_reserved_memory(void *blob, const char
> *basename,
> *ptr++ = cpu_to_fdt32(lower);
>
> /* store one or two size cells */
> - lower = fdt_size_unpack(carveout->end - carveout->start + 1, );
> + size = carveout->end - carveout->start + 1;
> + upper = upper_32_bits(size);
> + lower = lower_32_bits(size);
>
> if (ns > 1)
> *ptr++ = cpu_to_fdt32(upper);
> diff --git a/lib/fdtdec_test.c b/lib/fdtdec_test.c
> index f6defe16c5a6..1f4f27054057 100644
> --- a/lib/fdtdec_test.c
> +++ b/lib/fdtdec_test.c
> @@ -155,11 +155,13 @@ static int make_fdt_carveout_device(void *fdt, uint32_t
> na, uint32_t ns)
> };
> fdt32_t cells[4], *ptr = cells;
> uint32_t upper, lower;
> + fdt_size_t size;
> char name[32];
> int offset;
>
> /* store one or two address cells */
> - lower = fdt_addr_unpack(carveout.start, );
> + upper = upper_32_bits(carveout.start);
> + lower = lower_32_bits(carveout.start);
>
> if (na > 1 && upper > 0)
> snprintf(name, sizeof(name), "%s@%x,%x", basename, upper,
> @@ -173,7 +175,9 @@ static int make_fdt_carveout_device(void *fdt, uint32_t
> na, uint32_t ns)
> *ptr++ = cpu_to_fdt32(lower);
>
> /* store one or two size cells */
> - lower = fdt_size_unpack(carveout.end - carveout.start + 1, );
> + size = carveout.end - carveout.start + 1;
> + upper = upper_32_bits(size);
> + lower = lower_32_bits(size);
>
> if (ns > 1)
> *ptr++ = cpu_to_fdt32(upper);
> --
> 2.21.0
>
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 2/2] net: eth-uclass: Support device tree MAC addresses
From: Thierry Reding
Add the standard Ethernet device tree bindings (imported from v5.0 of
the Linux kernel) and implement support for reading the MAC address for
Ethernet devices in the Ethernet uclass. If the "mac-address" property
exists, the MAC address will be parsed from that. If that property does
not exist, the "local-mac-address" property will be tried as fallback.
MAC addresses from device tree take precedence over the ones stored in
a network interface card's ROM.
Acked-by: Joe Hershberger
Signed-off-by: Thierry Reding
---
Changes in v3:
- add additional check to make sure the MAC address read from device
tree is a valid MAC address
Changes in v2:
- use dev_read_u8_array_ptr()
.../devicetree/bindings/net/ethernet.txt | 66 +++
net/eth-uclass.c | 27 +++-
2 files changed, 90 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/ethernet.txt
diff --git a/Documentation/devicetree/bindings/net/ethernet.txt
b/Documentation/devicetree/bindings/net/ethernet.txt
new file mode 100644
index ..cfc376bc977a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ethernet.txt
@@ -0,0 +1,66 @@
+The following properties are common to the Ethernet controllers:
+
+NOTE: All 'phy*' properties documented below are Ethernet specific. For the
+generic PHY 'phys' property, see
+Documentation/devicetree/bindings/phy/phy-bindings.txt.
+
+- local-mac-address: array of 6 bytes, specifies the MAC address that was
+ assigned to the network device;
+- mac-address: array of 6 bytes, specifies the MAC address that was last used
by
+ the boot program; should be used in cases where the MAC address assigned to
+ the device by the boot program is different from the "local-mac-address"
+ property;
+- nvmem-cells: phandle, reference to an nvmem node for the MAC address;
+- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used;
+- max-speed: number, specifies maximum speed in Mbit/s supported by the device;
+- max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather than
+ the maximum frame size (there's contradiction in the Devicetree
+ Specification).
+- phy-mode: string, operation mode of the PHY interface. This is now a de-facto
+ standard property; supported values are:
+ * "internal"
+ * "mii"
+ * "gmii"
+ * "sgmii"
+ * "qsgmii"
+ * "tbi"
+ * "rev-mii"
+ * "rmii"
+ * "rgmii" (RX and TX delays are added by the MAC when required)
+ * "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, the
+ MAC should not add the RX or TX delays in this case)
+ * "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, the MAC
+ should not add an RX delay in this case)
+ * "rgmii-txid" (RGMII with internal TX delay provided by the PHY, the MAC
+ should not add an TX delay in this case)
+ * "rtbi"
+ * "smii"
+ * "xgmii"
+ * "trgmii"
+ * "2000base-x",
+ * "2500base-x",
+ * "rxaui"
+ * "xaui"
+ * "10gbase-kr" (10GBASE-KR, XFI, SFI)
+- phy-connection-type: the same as "phy-mode" property but described in the
+ Devicetree Specification;
+- phy-handle: phandle, specifies a reference to a node representing a PHY
+ device; this property is described in the Devicetree Specification and so
+ preferred;
+- phy: the same as "phy-handle" property, not recommended for new bindings.
+- phy-device: the same as "phy-handle" property, not recommended for new
+ bindings.
+- rx-fifo-depth: the size of the controller's receive fifo in bytes. This
+ is used for components that can have configurable receive fifo sizes,
+ and is useful for determining certain configuration settings such as
+ flow control thresholds.
+- tx-fifo-depth: the size of the controller's transmit fifo in bytes. This
+ is used for components that can have configurable fifo sizes.
+- managed: string, specifies the PHY management type. Supported values are:
+ "auto", "in-band-status". "auto" is the default, it usess MDIO for
+ management if fixed-link is not specified.
+
+Child nodes of the Ethernet controller are typically the individual PHY devices
+connected via the MDIO bus (sometimes the MDIO bus controller is separate).
+They are described in the phy.txt file in this same directory.
+For non-MDIO PHY management see fixed-link.txt.
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 4225aabf1fa1..28a82d7fc8d3 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -455,6 +455,23 @@ static int eth_pre_unbind(struct udevice *dev)
return 0;
}
+static bool eth_dev_get_mac_address(struct udevice *dev, u8 mac[ARP_HLEN])
+{
+ const uint8_t *p;
+
+
[U-Boot] [PATCH v3 1/2] net: eth-uclass: Write MAC address to hardware after probe
From: Thierry Reding In order for the device to use the proper MAC address, which can have been configured in the environment prior to the device being registered, ensure that the MAC address is written after the device has been probed. For devices that are registered before the network stack is initialized, this is already done during eth_initialize(). If the Ethernet device is on a bus that is not initialized on early boot, such as PCI, the device is not available at the time eth_initialize() is called, so we need the MAC address programming to also happen after probe. Acked-by: Joe Hershberger Signed-off-by: Thierry Reding --- net/eth-uclass.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 2ef20df19203..4225aabf1fa1 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -524,6 +524,8 @@ static int eth_post_probe(struct udevice *dev) #endif } + eth_write_hwaddr(dev); + return 0; } -- 2.21.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/2] net: eth-uclass: Support device tree MAC addresses
On Thu, Apr 18, 2019 at 07:30:05PM +0300, Grygorii Strashko wrote: > > > On 17.04.19 18:03, Thierry Reding wrote: > > On Wed, Apr 17, 2019 at 02:49:22PM +0300, Grygorii Strashko wrote: > >> > >> > >> On 16.04.19 19:24, Thierry Reding wrote: > >>> From: Thierry Reding > >>> > >>> Add the standard Ethernet device tree bindings (imported from v5.0 of > >>> the Linux kernel) and implement support for reading the MAC address for > >>> Ethernet devices in the Ethernet uclass. If the "mac-address" property > >>> exists, the MAC address will be parsed from that. If that property does > >>> not exist, the "local-mac-address" property will be tried as fallback. > >>> > >>> MAC addresses from device tree take precedence over the ones stored in > >>> a network interface card's ROM. > >>> > >>> Acked-by: Joe Hershberger > >>> Signed-off-by: Thierry Reding > >>> --- > >>> Changes in v2: > >>> - use dev_read_u8_array_ptr() > >>> > >>> .../devicetree/bindings/net/ethernet.txt | 66 +++ > >>> net/eth-uclass.c | 26 +++- > >>> 2 files changed, 89 insertions(+), 3 deletions(-) > >>> create mode 100644 Documentation/devicetree/bindings/net/ethernet.txt > >>> > >>> diff --git a/Documentation/devicetree/bindings/net/ethernet.txt > >>> b/Documentation/devicetree/bindings/net/ethernet.txt > >>> new file mode 100644 > >>> index ..cfc376bc977a > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/net/ethernet.txt > >>> @@ -0,0 +1,66 @@ > >>> +The following properties are common to the Ethernet controllers: > >>> + > >>> +NOTE: All 'phy*' properties documented below are Ethernet specific. For > >>> the > >>> +generic PHY 'phys' property, see > >>> +Documentation/devicetree/bindings/phy/phy-bindings.txt. > >>> + > >>> +- local-mac-address: array of 6 bytes, specifies the MAC address that was > >>> + assigned to the network device; > >>> +- mac-address: array of 6 bytes, specifies the MAC address that was last > >>> used by > >>> + the boot program; should be used in cases where the MAC address > >>> assigned to > >>> + the device by the boot program is different from the > >>> "local-mac-address" > >>> + property; > >>> +- nvmem-cells: phandle, reference to an nvmem node for the MAC address; > >>> +- nvmem-cell-names: string, should be "mac-address" if nvmem is to be > >>> used; > >>> +- max-speed: number, specifies maximum speed in Mbit/s supported by the > >>> device; > >>> +- max-frame-size: number, maximum transfer unit (IEEE defined MTU), > >>> rather than > >>> + the maximum frame size (there's contradiction in the Devicetree > >>> + Specification). > >>> +- phy-mode: string, operation mode of the PHY interface. This is now a > >>> de-facto > >>> + standard property; supported values are: > >>> + * "internal" > >>> + * "mii" > >>> + * "gmii" > >>> + * "sgmii" > >>> + * "qsgmii" > >>> + * "tbi" > >>> + * "rev-mii" > >>> + * "rmii" > >>> + * "rgmii" (RX and TX delays are added by the MAC when required) > >>> + * "rgmii-id" (RGMII with internal RX and TX delays provided by the > >>> PHY, the > >>> + MAC should not add the RX or TX delays in this case) > >>> + * "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, the > >>> MAC > >>> + should not add an RX delay in this case) > >>> + * "rgmii-txid" (RGMII with internal TX delay provided by the PHY, the > >>> MAC > >>> + should not add an TX delay in this case) > >>> + * "rtbi" > >>> + * "smii" > >>> + * "xgmii" > >>> + * "trgmii" > >>> + * "2000base-x", > >>> + * "2500base-x", > >>> + * "rxaui" > >>> + * "xaui" > >>> + * "10gbase-kr" (10GBASE-KR, XFI, SFI) &
Re: [U-Boot] [RESEND PATCH 1/2] net: rtl8169: Implement ->hwaddr_write() callback
On Tue, Apr 16, 2019 at 04:36:16PM +, Joe Hershberger wrote: > On Tue, Apr 16, 2019 at 11:21 AM Thierry Reding > wrote: > > > > From: Thierry Reding > > > > Implement this callback that allows the MAC address to be set for the > > Ethernet card. This is necessary in order for the device to be able to > > receive packets for the MAC address that U-Boot advertises. > > > > Signed-off-by: Thierry Reding > > Acked-by: Joe Hershberger Hi Joe, it's not clear to me who you expect to pick this (and patch 2/2) up. I didn't Cc anyone, so nobody else may consider themselves responsible for these. Did you mean to pick these up yourself or should they go via Simon's DT tree along with the two eth-uclass patches that I sent? Or perhaps TomR handles these patches directly? MAINTAINERS clearly identifies you as a maintainer for the u-boot-net tree, so I was expecting you to pick them up. Let me know if I should resend these to someone else with your Acked-by. Thierry signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] fdtdec: Use fdt_setprop_u32() for fdtdec_set_phandle()
On Mon, Apr 15, 2019 at 10:08:20AM +0200, Thierry Reding wrote:
> From: Thierry Reding
>
> The fdt_setprop_u32() function does everything that we need, so we
> really only use the function as a convenience wrapper, in which case it
> can simply be a static inline function.
>
> Signed-off-by: Thierry Reding
> ---
> include/fdtdec.h | 5 -
> lib/fdtdec.c | 7 ---
> 2 files changed, 4 insertions(+), 8 deletions(-)
Hi Simon,
gentle reminder on these. These are the two follow-up patches that you
had suggested I make on top of the other fdtdec series that you applied
a couple of weeks ago.
Thierry
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index 266c58271f0b..110aa6ab6dea 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -1029,7 +1029,10 @@ int fdtdec_setup_memory_banksize(void);
> * @param phandlephandle to set for the given node
> * @return 0 on success or a negative error code on failure
> */
> -int fdtdec_set_phandle(void *blob, int node, uint32_t phandle);
> +static inline int fdtdec_set_phandle(void *blob, int node, uint32_t phandle)
> +{
> + return fdt_setprop_u32(blob, node, "phandle", phandle);
> +}
>
> /**
> * fdtdec_add_reserved_memory() - add or find a reserved-memory node
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 9c9c30234732..fea44a9a8c65 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -1261,13 +1261,6 @@ __weak void *board_fdt_blob_setup(void)
> }
> #endif
>
> -int fdtdec_set_phandle(void *blob, int node, uint32_t phandle)
> -{
> - fdt32_t value = cpu_to_fdt32(phandle);
> -
> - return fdt_setprop(blob, node, "phandle", , sizeof(value));
> -}
> -
> static int fdtdec_init_reserved_memory(void *blob)
> {
> int na, ns, node, err;
> --
> 2.21.0
>
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/2] net: eth-uclass: Support device tree MAC addresses
On Wed, Apr 17, 2019 at 09:32:26PM -0700, Simon Glass wrote: > Hi Thierry, > > On Wed, 17 Apr 2019 at 08:03, Thierry Reding > wrote: > > > On Wed, Apr 17, 2019 at 02:49:22PM +0300, Grygorii Strashko wrote: > > > > > > > > > On 16.04.19 19:24, Thierry Reding wrote: > > > > From: Thierry Reding > > > > > > > > Add the standard Ethernet device tree bindings (imported from v5.0 of > > > > the Linux kernel) and implement support for reading the MAC address for > > > > Ethernet devices in the Ethernet uclass. If the "mac-address" property > > > > exists, the MAC address will be parsed from that. If that property does > > > > not exist, the "local-mac-address" property will be tried as fallback. > > > > > > > > MAC addresses from device tree take precedence over the ones stored in > > > > a network interface card's ROM. > > > > > > > > Acked-by: Joe Hershberger > > > > Signed-off-by: Thierry Reding > > > > --- > > > > Changes in v2: > > > > - use dev_read_u8_array_ptr() > > > > It would be good to have test cases for these. By "these", do you mean the eth_dev_get_mac_address() function that this patche introduces, or the dev_read_u8_array_ptr() function? For the former, that'd be a little difficult because it is not a public API, it's only called from the eth-uclass.c code. Thierry > > > > > > > > .../devicetree/bindings/net/ethernet.txt | 66 +++ > > > > net/eth-uclass.c | 26 +++- > > > > 2 files changed, 89 insertions(+), 3 deletions(-) > > > > create mode 100644 Documentation/devicetree/bindings/net/ethernet.txt > > > > Regards, > Simon signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/2] net: eth-uclass: Support device tree MAC addresses
On Wed, Apr 17, 2019 at 02:49:22PM +0300, Grygorii Strashko wrote: > > > On 16.04.19 19:24, Thierry Reding wrote: > > From: Thierry Reding > > > > Add the standard Ethernet device tree bindings (imported from v5.0 of > > the Linux kernel) and implement support for reading the MAC address for > > Ethernet devices in the Ethernet uclass. If the "mac-address" property > > exists, the MAC address will be parsed from that. If that property does > > not exist, the "local-mac-address" property will be tried as fallback. > > > > MAC addresses from device tree take precedence over the ones stored in > > a network interface card's ROM. > > > > Acked-by: Joe Hershberger > > Signed-off-by: Thierry Reding > > --- > > Changes in v2: > > - use dev_read_u8_array_ptr() > > > > .../devicetree/bindings/net/ethernet.txt | 66 +++ > > net/eth-uclass.c | 26 +++- > > 2 files changed, 89 insertions(+), 3 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/net/ethernet.txt > > > > diff --git a/Documentation/devicetree/bindings/net/ethernet.txt > > b/Documentation/devicetree/bindings/net/ethernet.txt > > new file mode 100644 > > index ..cfc376bc977a > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/ethernet.txt > > @@ -0,0 +1,66 @@ > > +The following properties are common to the Ethernet controllers: > > + > > +NOTE: All 'phy*' properties documented below are Ethernet specific. For the > > +generic PHY 'phys' property, see > > +Documentation/devicetree/bindings/phy/phy-bindings.txt. > > + > > +- local-mac-address: array of 6 bytes, specifies the MAC address that was > > + assigned to the network device; > > +- mac-address: array of 6 bytes, specifies the MAC address that was last > > used by > > + the boot program; should be used in cases where the MAC address assigned > > to > > + the device by the boot program is different from the "local-mac-address" > > + property; > > +- nvmem-cells: phandle, reference to an nvmem node for the MAC address; > > +- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used; > > +- max-speed: number, specifies maximum speed in Mbit/s supported by the > > device; > > +- max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather > > than > > + the maximum frame size (there's contradiction in the Devicetree > > + Specification). > > +- phy-mode: string, operation mode of the PHY interface. This is now a > > de-facto > > + standard property; supported values are: > > + * "internal" > > + * "mii" > > + * "gmii" > > + * "sgmii" > > + * "qsgmii" > > + * "tbi" > > + * "rev-mii" > > + * "rmii" > > + * "rgmii" (RX and TX delays are added by the MAC when required) > > + * "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, > > the > > + MAC should not add the RX or TX delays in this case) > > + * "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, the MAC > > + should not add an RX delay in this case) > > + * "rgmii-txid" (RGMII with internal TX delay provided by the PHY, the MAC > > + should not add an TX delay in this case) > > + * "rtbi" > > + * "smii" > > + * "xgmii" > > + * "trgmii" > > + * "2000base-x", > > + * "2500base-x", > > + * "rxaui" > > + * "xaui" > > + * "10gbase-kr" (10GBASE-KR, XFI, SFI) > > +- phy-connection-type: the same as "phy-mode" property but described in the > > + Devicetree Specification; > > +- phy-handle: phandle, specifies a reference to a node representing a PHY > > + device; this property is described in the Devicetree Specification and so > > + preferred; > > +- phy: the same as "phy-handle" property, not recommended for new bindings. > > +- phy-device: the same as "phy-handle" property, not recommended for new > > + bindings. > > +- rx-fifo-depth: the size of the controller's receive fifo in bytes. This > > + is used for components that can have configurable receive fifo sizes, > > + and is useful for determining certain configuration settings such as > > + flow control thresholds. > > +- tx-fifo-depth: the size o
Re: [U-Boot] [PATCH 1/2] pci: Add boundary check for hose->regions
On Fri, Mar 15, 2019 at 04:32:32PM +0100, Thierry Reding wrote:
> From: Thierry Reding
>
> Make sure that we don't overflow the hose->regions array, otherwise we
> would end up overwriting the hose->region_count field and cause mayhem
> to ensue. Also print an error message when we'd be overflowing because
> it indicates that there aren't enough regions available and the number
> needs to be increased.
>
> Signed-off-by: Thierry Reding
> ---
> drivers/pci/pci-uclass.c | 5 +
> 1 file changed, 5 insertions(+)
Hi Tom,
have you had a chance to look at these two patches? Simon's reviewed
them and they are needed to restore PCI support on Jetson TX2 (P2771).
Thierry
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index 824fa1190747..cf1e7617ae35 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -918,6 +918,11 @@ static void decode_regions(struct pci_controller *hose,
> ofnode parent_node,
> return;
>
> for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> + if (hose->region_count == MAX_PCI_REGIONS) {
> + pr_err("maximum number of regions parsed, aborting\n");
> + break;
> + }
> +
> if (bd->bi_dram[i].size) {
> pci_set_region(hose->regions + hose->region_count++,
> bd->bi_dram[i].start,
> --
> 2.20.1
>
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 2/2] net: eth-uclass: Support device tree MAC addresses
From: Thierry Reding
Add the standard Ethernet device tree bindings (imported from v5.0 of
the Linux kernel) and implement support for reading the MAC address for
Ethernet devices in the Ethernet uclass. If the "mac-address" property
exists, the MAC address will be parsed from that. If that property does
not exist, the "local-mac-address" property will be tried as fallback.
MAC addresses from device tree take precedence over the ones stored in
a network interface card's ROM.
Acked-by: Joe Hershberger
Signed-off-by: Thierry Reding
---
Changes in v2:
- use dev_read_u8_array_ptr()
.../devicetree/bindings/net/ethernet.txt | 66 +++
net/eth-uclass.c | 26 +++-
2 files changed, 89 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/ethernet.txt
diff --git a/Documentation/devicetree/bindings/net/ethernet.txt
b/Documentation/devicetree/bindings/net/ethernet.txt
new file mode 100644
index ..cfc376bc977a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ethernet.txt
@@ -0,0 +1,66 @@
+The following properties are common to the Ethernet controllers:
+
+NOTE: All 'phy*' properties documented below are Ethernet specific. For the
+generic PHY 'phys' property, see
+Documentation/devicetree/bindings/phy/phy-bindings.txt.
+
+- local-mac-address: array of 6 bytes, specifies the MAC address that was
+ assigned to the network device;
+- mac-address: array of 6 bytes, specifies the MAC address that was last used
by
+ the boot program; should be used in cases where the MAC address assigned to
+ the device by the boot program is different from the "local-mac-address"
+ property;
+- nvmem-cells: phandle, reference to an nvmem node for the MAC address;
+- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used;
+- max-speed: number, specifies maximum speed in Mbit/s supported by the device;
+- max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather than
+ the maximum frame size (there's contradiction in the Devicetree
+ Specification).
+- phy-mode: string, operation mode of the PHY interface. This is now a de-facto
+ standard property; supported values are:
+ * "internal"
+ * "mii"
+ * "gmii"
+ * "sgmii"
+ * "qsgmii"
+ * "tbi"
+ * "rev-mii"
+ * "rmii"
+ * "rgmii" (RX and TX delays are added by the MAC when required)
+ * "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, the
+ MAC should not add the RX or TX delays in this case)
+ * "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, the MAC
+ should not add an RX delay in this case)
+ * "rgmii-txid" (RGMII with internal TX delay provided by the PHY, the MAC
+ should not add an TX delay in this case)
+ * "rtbi"
+ * "smii"
+ * "xgmii"
+ * "trgmii"
+ * "2000base-x",
+ * "2500base-x",
+ * "rxaui"
+ * "xaui"
+ * "10gbase-kr" (10GBASE-KR, XFI, SFI)
+- phy-connection-type: the same as "phy-mode" property but described in the
+ Devicetree Specification;
+- phy-handle: phandle, specifies a reference to a node representing a PHY
+ device; this property is described in the Devicetree Specification and so
+ preferred;
+- phy: the same as "phy-handle" property, not recommended for new bindings.
+- phy-device: the same as "phy-handle" property, not recommended for new
+ bindings.
+- rx-fifo-depth: the size of the controller's receive fifo in bytes. This
+ is used for components that can have configurable receive fifo sizes,
+ and is useful for determining certain configuration settings such as
+ flow control thresholds.
+- tx-fifo-depth: the size of the controller's transmit fifo in bytes. This
+ is used for components that can have configurable fifo sizes.
+- managed: string, specifies the PHY management type. Supported values are:
+ "auto", "in-band-status". "auto" is the default, it usess MDIO for
+ management if fixed-link is not specified.
+
+Child nodes of the Ethernet controller are typically the individual PHY devices
+connected via the MDIO bus (sometimes the MDIO bus controller is separate).
+They are described in the phy.txt file in this same directory.
+For non-MDIO PHY management see fixed-link.txt.
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 4225aabf1fa1..c6d5ec013bd8 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -455,6 +455,23 @@ static int eth_pre_unbind(struct udevice *dev)
return 0;
}
+static bool eth_dev_get_mac_address(struct udevice *dev, u8 mac[ARP_HLEN])
+{
+ const uint8_t *p;
+
+ p = dev_read_u8_array_ptr(dev, "mac-address", ARP_HLEN);
+ if (!p)
+ p = dev_read_
[U-Boot] [PATCH v2 1/2] net: eth-uclass: Write MAC address to hardware after probe
From: Thierry Reding In order for the device to use the proper MAC address, which can have been configured in the environment prior to the device being registered, ensure that the MAC address is written after the device has been probed. For devices that are registered before the network stack is initialized, this is already done during eth_initialize(). If the Ethernet device is on a bus that is not initialized on early boot, such as PCI, the device is not available at the time eth_initialize() is called, so we need the MAC address programming to also happen after probe. Signed-off-by: Thierry Reding --- net/eth-uclass.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 2ef20df19203..4225aabf1fa1 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -524,6 +524,8 @@ static int eth_post_probe(struct udevice *dev) #endif } + eth_write_hwaddr(dev); + return 0; } -- 2.21.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [RESEND PATCH 2/2] net: rtl8169: Support RTL-8168h/8111h
From: Thierry Reding
This version of the RTL-8168 is present on some development boards and
is compatible with this driver. Add support for identifying this version
of the chip so that U-Boot won't complain about it being unknown.
Signed-off-by: Thierry Reding
---
drivers/net/rtl8169.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
index 27e27b34176b..bc052e72564b 100644
--- a/drivers/net/rtl8169.c
+++ b/drivers/net/rtl8169.c
@@ -257,6 +257,7 @@ static struct {
{"RTL-8168/8111g", 0x4c, 0xff7e1880,},
{"RTL-8101e", 0x34, 0xff7e1880,},
{"RTL-8100e", 0x32, 0xff7e1880,},
+ {"RTL-8168h/8111h", 0x54, 0xff7e1880,},
};
enum _DescStatusBit {
--
2.21.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [RESEND PATCH 1/2] net: rtl8169: Implement ->hwaddr_write() callback
From: Thierry Reding
Implement this callback that allows the MAC address to be set for the
Ethernet card. This is necessary in order for the device to be able to
receive packets for the MAC address that U-Boot advertises.
Signed-off-by: Thierry Reding
---
drivers/net/rtl8169.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
index a78f3d233f1a..27e27b34176b 100644
--- a/drivers/net/rtl8169.c
+++ b/drivers/net/rtl8169.c
@@ -941,6 +941,23 @@ static void rtl_halt(struct eth_device *dev)
}
#endif
+#ifdef CONFIG_DM_ETH
+static int rtl8169_write_hwaddr(struct udevice *dev)
+{
+ struct eth_pdata *plat = dev_get_platdata(dev);
+ unsigned int i;
+
+ RTL_W8(Cfg9346, Cfg9346_Unlock);
+
+ for (i = 0; i < MAC_ADDR_LEN; i++)
+ RTL_W8(MAC0 + i, plat->enetaddr[i]);
+
+ RTL_W8(Cfg9346, Cfg9346_Lock);
+
+ return 0;
+}
+#endif
+
/**
INIT - Look for an adapter, this routine's visible to the outside
***/
@@ -1195,6 +1212,7 @@ static const struct eth_ops rtl8169_eth_ops = {
.send = rtl8169_eth_send,
.recv = rtl8169_eth_recv,
.stop = rtl8169_eth_stop,
+ .write_hwaddr = rtl8169_write_hwaddr,
};
static const struct udevice_id rtl8169_eth_ids[] = {
--
2.21.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/3] net: eth-uclass: Write MAC address to hardware after probe
On Mon, Apr 15, 2019 at 09:24:00PM +, Joe Hershberger wrote: > On Mon, Apr 15, 2019 at 4:11 AM Thierry Reding > wrote: > > > > From: Thierry Reding > > > > In order for the device to use the proper MAC address, which can have > > been configured in the environment prior to the device being registered, > > ensure that the MAC address is written after the device has been probed. > > For devices that are registered before the network stack is initialized, > > this is already done during eth_initialize(). If the Ethernet device is > > on a bus that is not initialized on early boot, such as PCI, the device > > is not available at the time eth_initialize() is called, so we need the > > MAC address programming to also happen after probe. > > I would expect to also see a removal of the call in eth_initialize, > right? Why do it both places? I'm hesitant to do that. eth_initialize() happens after eth_post_probe() for devices that are on a fixed bus and there may be code setting up the MAC address that runs between eth_post_probe() and eth_initialize(). If we don't write the MAC address down to hardware in eth_initialize(), we may end up regressing those boards. Thierry > > Signed-off-by: Thierry Reding > > --- > > net/eth-uclass.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/net/eth-uclass.c b/net/eth-uclass.c > > index 2ef20df19203..4225aabf1fa1 100644 > > --- a/net/eth-uclass.c > > +++ b/net/eth-uclass.c > > @@ -524,6 +524,8 @@ static int eth_post_probe(struct udevice *dev) > > #endif > > } > > > > + eth_write_hwaddr(dev); > > + > > return 0; > > } > > > > -- > > 2.21.0 > > > > ___ > > U-Boot mailing list > > U-Boot@lists.denx.de > > https://lists.denx.de/listinfo/u-boot signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3] dm: core: Add dev_read_bytes()
On Mon, Apr 15, 2019 at 09:21:45PM +, Joe Hershberger wrote: > On Mon, Apr 15, 2019 at 4:11 AM Thierry Reding > wrote: > > > > From: Thierry Reding > > > > This function can be used to read a binary property into a buffer. One > > example where this is needed is to read a MAC address from device tree. > > > > Signed-off-by: Thierry Reding > > Is there a reason dev_read_u8_array_ptr is insuffient? No, it's perfectly suitable, I just hadn't seen it. Thierry signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
