Re: [PATCH 1/2] ARM: dts: am335x: Replace numeric pinmux address with macro defines
- Date: Thu, 14 Mar 2019 07:59:30 -0700
- From: Tony Lindgren <tony@xxxxxxxxxxx>
- Subject: Re: [PATCH 1/2] ARM: dts: am335x: Replace numeric pinmux address with macro defines
* Christina Quast <cquast@xxxxxxxxxxxxxxxxxxx> [190313 14:28]:
> The values are extraced from the "AM335x SitaraTM Processors Technical
> Reference Manual", Section 9.3.1 CONTROL_MODULE Registers, based on the
> file autogenerated by TI PinMux.
Thanks for updating this series. Few comments below.
> diff --git a/include/dt-bindings/pinctrl/am335x.h b/include/dt-bindings/pinctrl/am335x.h
> new file mode 100644
> index 000000000000..033a44efdc1e
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/am335x.h
I think these defines should be just added to the existing
include/dt-bindings/pinctrl/am33xx.h. That is assuming the
padconf registers are the same for all the variants.
> +#define PIN_MODE(mode) (mode)
> +#define PIN_PULL_UD_EN (0x1U << 3U)
> +#define PIN_PULL_TYPE_SEL (0x1U << 4U)
> +#define PIN_RX_ACTIVE (0x1U << 5U)
> +#define PIN_SLEW_SLOW (0x1U << 6U)
Hmm so in include/dt-bindings/pinctrl/am33xx.h we already have
these defined but with different names?
> +#define AM335X_PIN_OFFSET_MIN 0x0800U
You should leave out the generic control module registers
defines. So starting below..
> +#define AM335X_CONTROL_REVISION 0x0
> +#define AM335X_CONTROL_HWINFO 0x4
> +#define AM335X_CONTROL_SYSCONFIG 0x10
> +#define AM335X_CONTROL_STATUS 0x40
> +#define AM335X_CONTROL_EMIF_SDRAM_CONFIG 0x110
> +#define AM335X_BB_SCALE 0x7d0
> +#define AM335X_USB_VID_PID 0x7f4
> +#define AM335X_EFUSE_SMA 0x7fc
.. all the way here. This header should only have the
padconf area registers that should all have PIN in the
name. So only keep the ones from below..
> +#define AM335X_PIN_GPMC_AD0 0x800
> +#define AM335X_PIN_GPMC_AD1 0x804
> +#define AM335X_PIN_GPMC_AD2 0x808
> +#define AM335X_PIN_USB0_DRVVBUS 0xa1c
> +#define AM335X_PIN_USB1_DRVVBUS 0xa34
.. to here. Then also drop the defines from here..
> +#define AM335X_CQDETECT_STATUS 0xe00
> +#define AM335X_DDR_IO_CTRL 0xe04
> +#define AM335X_VTP_CTRL 0xe0c
> +#define AM335X_DDR_CMD2_IOCTRL 0x140c
> +#define AM335X_DDR_DATA0_IOCTRL 0x1440
> +#define AM335X_DDR_DATA1_IOCTRL 0x1444
.. to here.
> +#define AM335X_PIN_OFFSET_MAX 0x1320U
And then adjust the AM335X_PIN_OFFSET_MAX accordingly
if that is needed.
Note that the padconf range is specified in am33xx-l4.dtsi
for pinmux@800 in the reg range so this header should
contain the same registers. Some SoCs have multiple padconf
ranges but am335x only has one.