Hi Sean,
----- Цитат от Sean Cross (xobs@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