On Wed, 2 Aug 2023 at 22:09, Jean-Christophe Dubois <j...@tribudubois.net>
wrote:
>
> * Add Addr and size definition for all i.MX7 devices in i.MX7 header file.
> * Use those newly defined named constants whenever possible.
> * Standardize the way we init a familly of unimplemented devices
> - SAI
> - PWM
> - CAN
> * Add/rework few comments
>
> Signed-off-by: Jean-Christophe Dubois <j...@tribudubois.net>
> ---
> /*
> - * IOMUXC and IOMUXC_LPSR
> + * IOMUXC, IOMUXC_GPR and IOMUXC_LPSR
> */
> - for (i = 0; i < FSL_IMX7_NUM_IOMUXCS; i++) {
> - static const hwaddr FSL_IMX7_IOMUXCn_ADDR[FSL_IMX7_NUM_IOMUXCS] = {
> - FSL_IMX7_IOMUXC_ADDR,
> - FSL_IMX7_IOMUXC_LPSR_ADDR,
> - };
> -
> - snprintf(name, NAME_SIZE, "iomuxc%d", i);
> - create_unimplemented_device(name, FSL_IMX7_IOMUXCn_ADDR[i],
> - FSL_IMX7_IOMUXCn_SIZE);
> - }
> + create_unimplemented_device("iomuxc", FSL_IMX7_IOMUXC_ADDR,
> + FSL_IMX7_IOMUXC_SIZE);
> + create_unimplemented_device("iomuxc_gpr", FSL_IMX7_IOMUXC_GPR_ADDR,
> + FSL_IMX7_IOMUXC_GPR_SIZE);
> + create_unimplemented_device("iomuxc_lspr", FSL_IMX7_IOMUXC_LPSR_ADDR,
> + FSL_IMX7_IOMUXC_LPSR_SIZE);
This is a behaviour change -- we used to create 2 stub
iomux devices, and now we create 3. Also, we map the
iomuxc_gpr at FSL_IMX7_IOMUXC_GPR_ADDR here, but we also do
sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpr), 0, FSL_IMX7_IOMUXC_GPR_ADDR);
below. Which is correct ?
> create_unimplemented_device("caam", FSL_IMX7_CAAM_ADDR,
> FSL_IMX7_CAAM_SIZE);
>
> /*
> - * PWM
> + * SAIs (Audio SSI (Synchronous Serial Interface))
> */
> - create_unimplemented_device("pwm1", FSL_IMX7_PWM1_ADDR,
> FSL_IMX7_PWMn_SIZE);
> - create_unimplemented_device("pwm2", FSL_IMX7_PWM2_ADDR,
> FSL_IMX7_PWMn_SIZE);
> - create_unimplemented_device("pwm3", FSL_IMX7_PWM3_ADDR,
> FSL_IMX7_PWMn_SIZE);
> - create_unimplemented_device("pwm4", FSL_IMX7_PWM4_ADDR,
> FSL_IMX7_PWMn_SIZE);
> + for (i = 0; i < FSL_IMX7_NUM_SAIS; i++) {
> + static const hwaddr FSL_IMX7_SAIn_ADDR[FSL_IMX7_NUM_SAIS] = {
> + FSL_IMX7_SAI1_ADDR,
> + FSL_IMX7_SAI2_ADDR,
> + FSL_IMX7_SAI3_ADDR,
> + };
> +
> + snprintf(name, NAME_SIZE, "sai%d", i);
> + create_unimplemented_device(name, FSL_IMX7_SAIn_ADDR[i],
> + FSL_IMX7_SAIn_SIZE);
> + }
Any reason for moving the SAI device creation up like this? It
makes the diff confusing.
>
> /*
> - * CAN
> + * PWMs
> */
> - create_unimplemented_device("can1", FSL_IMX7_CAN1_ADDR,
> FSL_IMX7_CANn_SIZE);
> - create_unimplemented_device("can2", FSL_IMX7_CAN2_ADDR,
> FSL_IMX7_CANn_SIZE);
> + for (i = 0; i < FSL_IMX7_NUM_PWMS; i++) {
> + static const hwaddr FSL_IMX7_PWMn_ADDR[FSL_IMX7_NUM_PWMS] = {
> + FSL_IMX7_PWM1_ADDR,
> + FSL_IMX7_PWM2_ADDR,
> + FSL_IMX7_PWM3_ADDR,
> + FSL_IMX7_PWM4_ADDR,
> + };
> +
> + snprintf(name, NAME_SIZE, "pwm%d", i);
> + create_unimplemented_device(name, FSL_IMX7_PWMn_ADDR[i],
> + FSL_IMX7_PWMn_SIZE);
> + }
>
> /*
> - * SAI (Audio SSI (Synchronous Serial Interface))
> + * CANs
> */
> - create_unimplemented_device("sai1", FSL_IMX7_SAI1_ADDR,
> FSL_IMX7_SAIn_SIZE);
> - create_unimplemented_device("sai2", FSL_IMX7_SAI2_ADDR,
> FSL_IMX7_SAIn_SIZE);
> - create_unimplemented_device("sai2", FSL_IMX7_SAI3_ADDR,
> FSL_IMX7_SAIn_SIZE);
> + for (i = 0; i < FSL_IMX7_NUM_CANS; i++) {
> + static const hwaddr FSL_IMX7_CANn_ADDR[FSL_IMX7_NUM_CANS] = {
> + FSL_IMX7_CAN1_ADDR,
> + FSL_IMX7_CAN2_ADDR,
> + };
> +
> + snprintf(name, NAME_SIZE, "can%d", i);
> + create_unimplemented_device(name, FSL_IMX7_CANn_ADDR[i],
> + FSL_IMX7_CANn_SIZE);
> + }
thanks
-- PMM
