Hi,

* Christina Quast <cqu...@hanoverdisplays.com> [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.

Regards,

Tony

Reply via email to