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

picmaster at mail.bg
2014-10-08T15:35:44Z

Hi Sean,

----- ????? ?? Sean Cross (xobs at kosagi.com), ?? 08.10.2014 ? 10:47 -----

> 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?

@Sean: Do you have any comments on USB hub reset stuff? Should we ditch
entirely the reset code, or should we modify it somehow to work properly?

>>> +#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.

Well, that would be great if it works out this way. I just shared what
happened with another audio codec, to spare some frustration to others.

> 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.

Agreed.

> 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.

Well, all I have is the Novena schematic (http://bunniefoo.com/novena/pvt2_release/novena_pvt2.PDF) so all my analysis is based on this PDF. On page 14, R30A is
DNP, and R28A is populated, this is why I suggested this modification to enable
speaker amps. If there are schematic changes, please publish the new version
so we can use the proper schematic and reduce spam.

@Marek: According to this latest update, there's no need for my fix for
NOVENA_AUDIO_SPK_AMP_ON, as your original code will work just fine with the
planned schematic changes. Please disregard the fix.

Kind regards,
Nikolay