[U-Boot] [PATCH V5] ARM: mx6: Add support for Kosagi Novena

Sean Cross
2014-10-08T07:47:43Z

On 08/10/2014 05:55, Nikolay Dimitrov wrote:
> Hi Marek,
>
> I'm marking only the critical issues that are left unfixed from
> previous conversations, to speed-up the process a little bit.
> I'll send later patches for the non-critical issues to spare you the
> extra work (and I'm sure my constructive criticism is already boring
> :D ).
>
>
> On 10/06/2014 07:02 PM, Marek Vasut wrote:
>> +#define NOVENA_USB_HUB_RESET IMX_GPIO_NR(7, 12)
> ...
>> +/*
>> + * USB
>> + */
>> +#ifdef CONFIG_USB_EHCI_MX6
>> +int board_ehci_hcd_init(int port)
>> +{
>> + /* Reset USB hub */
>> + if (port == 1) {
>> + gpio_set_value(NOVENA_USB_HUB_RESET, 0);
>> + mdelay(2);
>> + gpio_set_value(NOVENA_USB_HUB_RESET, 1);
>> + }
>> + return 0;
>> +}
>> +#endif
>
> As we previously discussed, this pin definition conflicts with
> NOVENA_PCIE_POWER_ON_GPIO (GPIO7_IO12 is connected to PCIE_PWRON), so
> by asserting it, you'll turn-off the wrong sub-system.
>
> Currently the USB hub is reset only by system reset (RESETBMCU
> asserted by the PMIC). I don't see how the CPU can selectively reset
> the USB hub via a GPIO, so it would be better to remove the reset code.
>
> @Sean - can you please confirm/reject this finding?
>
>
>> +#define NOVENA_AUDIO_PWRON IMX_GPIO_NR(5, 17)
> ...
>> +/*
>> + * Audio
>> + */
>> +static iomux_v3_cfg_t audio_pads[] = {
>> + /* AUD_PWRON */
>> + MX6_PAD_DISP0_DAT23__GPIO5_IO17 | MUX_PAD_CTRL(NO_PAD_CTRL),
>> +};
>
> The speaker amplifiers will still be disabled, as AUDIO_PWRON is not
> connected to them (R30A is marked as DNP). You need to add one more
> GPIO to enable the speaker amplifiers, here's the fix:
>
> #define NOVENA_AUDIO_PWRON IMX_GPIO_NR(5, 17)
> #define NOVENA_AUDIO_SPK_AMP_ON IMX_GPIO_NR(4, 9)
>
> /*
> * Audio
> */
> static iomux_v3_cfg_t audio_pads[] = {
> /* AUD_PWRON */
> MX6_PAD_KEY_ROW1__GPIO5_IO17 | MUX_PAD_CTRL(NO_PAD_CTRL),
>
> /* Speakers' amplifiers #SHDWN */
> MX6_PAD_KEY_ROW1__GPIO4_IO09 | MUX_PAD_CTRL(NO_PAD_CTRL),
> };
>
> static void novena_spl_setup_iomux_audio(void)
> {
> imx_iomux_v3_setup_multiple_pads(audio_pads, ARRAY_SIZE(audio_pads));
>
> gpio_direction_output(NOVENA_AUDIO_PWRON, 1);
> gpio_direction_output(NOVENA_AUDIO_SPK_AMP_ON, 1);
> }
>
>
> Side comment: If someone needs to talk to the Audio Codec via I2C3 in
> U-Boot environment, don't be surprised if the codec doesn't respond to
> any I2C requests at all (I've had such issues with SGTL5000). The most
> probable reason is that the codec doesn't receive reference clock from
> imx6 GPIO0. This is not a critical issues as later the kernel muxes
> this GPIO0 properly, but keep this in mind if you want to hack the
> audio-codec at low level.

The audio codec ought to answer even when the reference clock isn't
active. I actually can't recall, but I think the digital side still
works. The audio codec needs to be turned on because otherwise it will
pull the SCL and SCK lines low, blocking I2C3 from working at all.

Actually, now that I'm looking at it, we just had to make an ECO to get
rid of SPK_AMP_ON. It is now ganged to AUDIO_PWRON, because SPK_AMP_ON
was accidentally dual-purposed as a GPIO for the front button on the
desktop / laptop. This change was made very recently (last Friday I
think), and we'll be switching resistors on the mainboard when they're
built in order to make the change. That pin should still be muxed as a
GPIO, it's just an input now rather than an output.

Sean