> -----Original Message----- > From: Abel Vesa [mailto:abel.vesa@xxxxxxx] > Sent: Tuesday, October 27, 2020 7:55 PM > To: Lucas Stach <l.stach@xxxxxxxxxxxxxx> > Cc: Adam Ford <aford173@xxxxxxxxx>; Marek Vasut <marex@xxxxxxx>; > devicetree <devicetree@xxxxxxxxxxxxxxx>; Sascha Hauer > <s.hauer@xxxxxxxxxxxxxx>; Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>; > Stephen Boyd <sboyd@xxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>; > Michael Turquette <mturquette@xxxxxxxxxxxx>; Linux Kernel Mailing List > <linux-kernel@xxxxxxxxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; > dl-linux-imx <linux-imx@xxxxxxx>; Pengutronix Kernel Team > <kernel@xxxxxxxxxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>; linux-clk > <linux-clk@xxxxxxxxxxxxxxx>; arm-soc <linux-arm-kernel@xxxxxxxxxxxxxxxxxxx> > Subject: Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN > > On 20-10-27 11:31:10, Abel Vesa wrote: > > On 20-10-26 16:37:51, Lucas Stach wrote: > > > Am Montag, den 26.10.2020, 16:55 +0200 schrieb Abel Vesa: > > > > On 20-10-25 11:05:32, Adam Ford wrote: > > > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@xxxxxxx> > wrote: > > > > > > On 10/25/20 1:05 PM, Abel Vesa wrote: > > > > > > > > > > > > [...] > > > > > > > > > > > > > > Together, both the GPC and the clk-blk driver should be > > > > > > > > able to pull the multimedia block out of reset. > > > > > > > > Currently, the GPC can handle the USB OTG and the GPU, but > > > > > > > > the LCDIF and MIPI DSI appear to be gated by the clock > > > > > > > > block > > > > > > > > > > > > > > > > My original patch RFC didn't include the imx8mn node, > > > > > > > > because it hangs, but the node I added looks like: > > > > > > > > > > > > > > > > media_blk_ctl: clock-controller@32e28000 { > > > > > > > > compatible = "fsl,imx8mn-media-blk-ctl", "syscon"; > > > > > > > > reg = <0x32e28000 0x1000>; > > > > > > > > #clock-cells = <1>; > > > > > > > > #reset-cells = <1>; > > > > > > > > }; > > > > > > > > > > > > > > > > I was hoping you might have some feedback on the 8mn > > > > > > > > clk-blk driver since you did the 8mp clk-blk drive and > > > > > > > > they appear to be very similar. > > > > > > > > > > > > > > > > > > > > > > I'll do you one better still. I'll apply the patch in my > > > > > > > tree and give it a test tomorrow morning. > > > > > > > > > > I do have some more updates on how to get the system to not > > > > > hang, and to enumerate more clocks. > > > > > Looking at Marek's work on enabling clocks in the 8MM, he added > > > > > a power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC. > > > > > By forcing the GPC driver to write 0x1fff to 32e28004, 0x7f to > > > > > 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the > > > > > display clocks out of reset. > > > > > > > > > > > > > Yeah, that makes sense. Basically, it was trying to disable unused > > > > clocks (see clk_disable_unused) but in order to disable the clocks > > > > from the media BLK_CTL (which I think should be renamed in display > > > > BLK_CTL) the PD need to be on. Since you initially didn't give it > > > > any PD, it was trying to blindly write/read the gate bit and therefore > freeze. > > > > > > > > > Unfortunately, the i.MX8MN needs to have 0x100 written to both > > > > > 32e28000 and 32e28004, and the values written for the 8MM are > > > > > not compatible. > > > > > By forcing the GPC to write those values, I can get > > > > > lcdif_pixel_clk and the mipi_dsi_clkref appearing on the Nano. > > > > > > > > I'm trying to make a branch with all the patches for all i.MX8M so > > > > I can keep track of it all. On this branch I've also applied the > > > > following patchset from Lucas Stach: > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > > > > www.spinics.net%2Flists%2Farm-kernel%2Fmsg843007.html&data=04% > > > > > 7C01%7Cping.bai%40nxp.com%7C0c54623294334a04a01208d87a6f3163%7 > C686 > > > > > ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637393965282215014%7C > Unkno > > > > > wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h > a > > > > > WwiLCJXVCI6Mn0%3D%7C1000&sdata=vFbBn94CPsShV72rOCtbTcz5u0 > qh7Au > > > > o44Sb2%2BiBlrE%3D&reserved=0 but I'm getting the folowing > > > > errors: > > > > > > > > [ 16.690885] imx-pgc imx-pgc-domain.3: failed to power up ADB400 > > > > [ 16.716839] imx-pgc imx-pgc-domain.3: failed to power up ADB400 > > > > [ 16.730500] imx-pgc imx-pgc-domain.3: failed to power up ADB400 > > > > > > > > Lucas, any thoughts? > > > > > > > > Maybe it's something related to 8MN. > > > > > > The ADB is apparently clocked by one of the BLK_CTL clocks, so the > > > ADB handshake ack will only work when the BLK_CTL clocks are > > > enabled. So I guess the GPC driver should enable those clocks and > > > assert the resets at the right time in the power-up sequencing. > > > Unfortunately this means we can't properly put the BLK_CTL driver in > > > the power-domain without having a cyclic dependency in the DT. I'm > > > still thinking about how to solve this properly. > > > > > > > I remember we had something similar in our internal tree with the > > bus_blk_clk on 8MP, which was added by the media BLK_CTL. What I did > > was to just drop the registration of that clock entirely. My rationale > > was that if the clock is part of the BLK_CTL but also needed by the > > BLK_CTL to work, I can leave it alone (that is, enabled by default) > > since when the PD will be powered off the clock will gated too. I > > guess another option would be to mark it as critical, that way, it > > will never be disabled (will be left alone by the clk_disable_unused > > too) but at the same time will be visible in the clock hierarchy. > > > > Do ignore evrything I said about the bus_blk_ctl, that did work on our tree > since the whole PD power on/off "magic" is done in TF-A. > > So the problem, as I understand it now, is the fact that the blk_ctl driver won't > probe because it needs its PD, but the PD is not registered because the > ADB400 can't power up since it needs the bus_blk_ctl clock enabled, clock > which is registered by the blk_ctl. 1. For some MIX's BLK_CTL GPRs, it can only be accessed when the MIX PD is on 2. for some MIX's ADB handshake, need to config some BLK_CTL clock_en bit to enable the MIX internal bus clock. That's why I have concern on implementing such MIX GPR as clock & reset driver, and implementing GPC PD in linux kernel. It will introduce some chicken-egg issue that not easy to handle in linux kernel. BR Jacky Bai > > > > Regards, > > > Lucas > > >
