Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-29 Thread Lucas Stach
Am Donnerstag, den 29.10.2020, 07:18 -0500 schrieb Adam Ford:
> On Thu, Oct 29, 2020 at 6:55 AM Lucas Stach 
> wrote:
> > Am Montag, den 26.10.2020, 11:23 -0500 schrieb Adam Ford:
> > > On Mon, Oct 26, 2020 at 10:44 AM Lucas Stach <
> > > l.st...@pengutronix.de> wrote:
> > > > Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> > > > > On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa 
> > > > > wrote:
> > > > > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <
> > > > > > > ma...@denx.de> 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 0x3 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://www.spinics.net/lists/arm-kernel/msg843007.html
> > > > > > 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.
> > > > > > 
> > > > > I will go back and double check this now that we have both
> > > > > the
> > > > > blt_crl->power-domain and the power-domain->blk_ctl.
> > > > > 
> > > > > > Will dig further, see what pops out.
> > > > > 
> > > > > I wasn't sure which direction to go with the name.  I can
> > > > > rename the
> > > > > media_blk_ctl  driver to display_blk_ctl.  I used Media based
> > > > > on the
> > > > > imx8mp naming convention and the fact that it's controlling
> > > > > both the
> > > > > display and the camera interface, however it's depending on
> > > > > the
> > > > > dispmix GPC.
> > > > > 
> > > > > I'll submit a RFC V2 with the cross referencing to the GPC
> > > > > based on
> > > > > Marek's Mini patch set, but we'll still have an issue where
> > > > > the Mini
> > > > > 

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-29 Thread Adam Ford
On Thu, Oct 29, 2020 at 6:55 AM Lucas Stach  wrote:
>
> Am Montag, den 26.10.2020, 11:23 -0500 schrieb Adam Ford:
> > On Mon, Oct 26, 2020 at 10:44 AM Lucas Stach  wrote:
> > > Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> > > > On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa  wrote:
> > > > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut  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 0x3 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://www.spinics.net/lists/arm-kernel/msg843007.html
> > > > > 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.
> > > > >
> > > > I will go back and double check this now that we have both the
> > > > blt_crl->power-domain and the power-domain->blk_ctl.
> > > >
> > > > > Will dig further, see what pops out.
> > > >
> > > > I wasn't sure which direction to go with the name.  I can rename the
> > > > media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
> > > > imx8mp naming convention and the fact that it's controlling both the
> > > > display and the camera interface, however it's depending on the
> > > > dispmix GPC.
> > > >
> > > > I'll submit a RFC V2 with the cross referencing to the GPC based on
> > > > Marek's Mini patch set, but we'll still have an issue where the Mini
> > > > and Nano have different syscon values to enable the clocks, and
> > > > Marek's branch has it card-coded, so my patch would effectively break
> > > > the Mini in order to make the Nano operate until we find a better
> > > > solution.
> > >
> > > The GPC should not write into the BLK_CTL region via syscon, but
> > > instead use the clocks and resets as exposed by the BLK_CTL driver.
> > > Doing it via syscon is a hack to get things going. The clocks and
> > > resets should properly be hooked up to the GPC domains via the clocks
> > > and resets DT properties.
> > >
> > > For the clocks there is one complication: 

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-29 Thread Abel Vesa
On 20-10-29 12:54:58, Lucas Stach wrote:
> Am Montag, den 26.10.2020, 11:23 -0500 schrieb Adam Ford:
> > On Mon, Oct 26, 2020 at 10:44 AM Lucas Stach  wrote:
> > > Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> > > > On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa  wrote:
> > > > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut  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 0x3 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%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg843007.htmldata=04%7C01%7Cabel.vesa%40nxp.com%7C7ee028d464cf451e0e2d08d87c0177cd%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637395693031971288%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=yKGtS%2FTyKjGHZ2xcXSI8%2F74R9vUjPmXgT9FSQfUfZB4%3Dreserved=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.
> > > > > 
> > > > I will go back and double check this now that we have both the
> > > > blt_crl->power-domain and the power-domain->blk_ctl.
> > > > 
> > > > > Will dig further, see what pops out.
> > > > 
> > > > I wasn't sure which direction to go with the name.  I can rename the
> > > > media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
> > > > imx8mp naming convention and the fact that it's controlling both the
> > > > display and the camera interface, however it's depending on the
> > > > dispmix GPC.
> > > > 
> > > > I'll submit a RFC V2 with the cross referencing to the GPC based on
> > > > Marek's Mini patch set, but we'll still have an issue where the Mini
> > > > and Nano have different syscon values to enable the clocks, and
> > > > Marek's branch has it card-coded, so my patch would effectively break
> > > > the Mini in order to make the Nano operate until we find a better
> > > > solution.
> > > 
> > > The 

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-29 Thread Lucas Stach
Am Montag, den 26.10.2020, 11:23 -0500 schrieb Adam Ford:
> On Mon, Oct 26, 2020 at 10:44 AM Lucas Stach  wrote:
> > Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> > > On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa  wrote:
> > > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut  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 0x3 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://www.spinics.net/lists/arm-kernel/msg843007.html
> > > > 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.
> > > > 
> > > I will go back and double check this now that we have both the
> > > blt_crl->power-domain and the power-domain->blk_ctl.
> > > 
> > > > Will dig further, see what pops out.
> > > 
> > > I wasn't sure which direction to go with the name.  I can rename the
> > > media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
> > > imx8mp naming convention and the fact that it's controlling both the
> > > display and the camera interface, however it's depending on the
> > > dispmix GPC.
> > > 
> > > I'll submit a RFC V2 with the cross referencing to the GPC based on
> > > Marek's Mini patch set, but we'll still have an issue where the Mini
> > > and Nano have different syscon values to enable the clocks, and
> > > Marek's branch has it card-coded, so my patch would effectively break
> > > the Mini in order to make the Nano operate until we find a better
> > > solution.
> > 
> > The GPC should not write into the BLK_CTL region via syscon, but
> > instead use the clocks and resets as exposed by the BLK_CTL driver.
> > Doing it via syscon is a hack to get things going. The clocks and
> > resets should properly be hooked up to the GPC domains via the clocks
> > and resets DT properties.
> > 
> > For the clocks there is one complication: if the clocks are controlled
> > via BLK_CTL we can only enable them once the domain is powered up,
> > however the earlier designs using the GPCv2 assert resets as part of
> > the power up sequence, which needs the clocks to be running for the
> > reset to 

RE: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-28 Thread Jacky Bai
> -Original Message-
> From: Abel Vesa [mailto:abel.v...@nxp.com]
> Sent: Tuesday, October 27, 2020 7:55 PM
> To: Lucas Stach 
> Cc: Adam Ford ; Marek Vasut ;
> devicetree ; Sascha Hauer
> ; Philipp Zabel ;
> Stephen Boyd ; Fabio Estevam ;
> Michael Turquette ; Linux Kernel Mailing List
> ; Rob Herring ;
> dl-linux-imx ; Pengutronix Kernel Team
> ; Shawn Guo ; linux-clk
> ; arm-soc 
> 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 
> 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 0x3 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.htmldata=04%
> > > >
> 7C01%7Cping.bai%40nxp.com%7C0c54623294334a04a01208d87a6f3163%7
> C686
> > > >
> ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637393965282215014%7C
> Unkno
> > > >
> wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> a
> > > >
> WwiLCJXVCI6Mn0%3D%7C1000sdata=vFbBn94CPsShV72rOCtbTcz5u0
> qh7Au
> > > > o44S

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-27 Thread Abel Vesa
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  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 0x3 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%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg843007.htmldata=04%7C01%7Cabel.vesa%40nxp.com%7C5ff46189143747fce45908d87a5b4281%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637393879674506099%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=ELDCbfLvxrB6FLwnsA6VyGlU5V3qpA2ImfPAbZnWyzI%3Dreserved=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

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-27 Thread Abel Vesa
On 20-10-26 16:44:13, Lucas Stach wrote:
> Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> > On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa  wrote:
> > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut  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 0x3 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%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg843007.htmldata=04%7C01%7Cabel.vesa%40nxp.com%7C02bcfb84d35f4c41a05e08d879c5fe57%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637393238611075979%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=fQnrRpGOnk0B4tFCFsfadKK2qozZwxK4ddycmv4VPls%3Dreserved=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.
> > > 
> > I will go back and double check this now that we have both the
> > blt_crl->power-domain and the power-domain->blk_ctl.
> > 
> > > Will dig further, see what pops out.
> > 
> > I wasn't sure which direction to go with the name.  I can rename the
> > media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
> > imx8mp naming convention and the fact that it's controlling both the
> > display and the camera interface, however it's depending on the
> > dispmix GPC.
> > 
> > I'll submit a RFC V2 with the cross referencing to the GPC based on
> > Marek's Mini patch set, but we'll still have an issue where the Mini
> > and Nano have different syscon values to enable the clocks, and
> > Marek's branch has it card-coded, so my patch would effectively break
> > the Mini in order to make the Nano operate until we find a better
> > solution.
> 
> The GPC should not write into the BLK_CTL region via syscon, but
> instead use the clocks and resets as exposed by the BLK_CTL driver.
> Doing it via syscon is a hack to get things going. The clocks and
> resets should properly be hooked up to the GPC domains via the clocks
> and resets DT properties.
> 

Totally agree. The syscon is there for other GPRs of any BLK_CTL which
do not fit in a clock or reset controller. So please do not ever use the
syscon for touching clocks or reset. Actually, I'm thinking of a way to
make sure the 

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-27 Thread Abel Vesa
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  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 0x3 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%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg843007.htmldata=04%7C01%7Cabel.vesa%40nxp.com%7Cc930b0f523c44946a93f08d879c51c3f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637393234789815116%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=ZzzUpEiEVcWqQQ5azAx8dkjHgiSMwkK04tqi32uLKbU%3Dreserved=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.

> Regards,
> Lucas
> 


Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-26 Thread Adam Ford
On Mon, Oct 26, 2020 at 10:44 AM Lucas Stach  wrote:
>
> Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> > On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa  wrote:
> > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut  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 0x3 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://www.spinics.net/lists/arm-kernel/msg843007.html
> > > 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.
> > >
> > I will go back and double check this now that we have both the
> > blt_crl->power-domain and the power-domain->blk_ctl.
> >
> > > Will dig further, see what pops out.
> >
> > I wasn't sure which direction to go with the name.  I can rename the
> > media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
> > imx8mp naming convention and the fact that it's controlling both the
> > display and the camera interface, however it's depending on the
> > dispmix GPC.
> >
> > I'll submit a RFC V2 with the cross referencing to the GPC based on
> > Marek's Mini patch set, but we'll still have an issue where the Mini
> > and Nano have different syscon values to enable the clocks, and
> > Marek's branch has it card-coded, so my patch would effectively break
> > the Mini in order to make the Nano operate until we find a better
> > solution.
>
> The GPC should not write into the BLK_CTL region via syscon, but
> instead use the clocks and resets as exposed by the BLK_CTL driver.
> Doing it via syscon is a hack to get things going. The clocks and
> resets should properly be hooked up to the GPC domains via the clocks
> and resets DT properties.
>
> For the clocks there is one complication: if the clocks are controlled
> via BLK_CTL we can only enable them once the domain is powered up,
> however the earlier designs using the GPCv2 assert resets as part of
> the power up sequence, which needs the clocks to be running for the
> reset to propagate. So depending on whether we have a power domain with
> a BLK_CTL or not we need to enable the clocks before or after powering
> up the domain. I guess we need a new DT property to specify which way
> the domain needs to handled.

So in the case of Nano, could we create two blocks instead of 

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-26 Thread Lucas Stach
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  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 0x3 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://www.spinics.net/lists/arm-kernel/msg843007.html
> 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.

Regards,
Lucas



Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-26 Thread Lucas Stach
Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa  wrote:
> > On 20-10-25 11:05:32, Adam Ford wrote:
> > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut  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 0x3 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://www.spinics.net/lists/arm-kernel/msg843007.html
> > 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.
> > 
> I will go back and double check this now that we have both the
> blt_crl->power-domain and the power-domain->blk_ctl.
> 
> > Will dig further, see what pops out.
> 
> I wasn't sure which direction to go with the name.  I can rename the
> media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
> imx8mp naming convention and the fact that it's controlling both the
> display and the camera interface, however it's depending on the
> dispmix GPC.
> 
> I'll submit a RFC V2 with the cross referencing to the GPC based on
> Marek's Mini patch set, but we'll still have an issue where the Mini
> and Nano have different syscon values to enable the clocks, and
> Marek's branch has it card-coded, so my patch would effectively break
> the Mini in order to make the Nano operate until we find a better
> solution.

The GPC should not write into the BLK_CTL region via syscon, but
instead use the clocks and resets as exposed by the BLK_CTL driver.
Doing it via syscon is a hack to get things going. The clocks and
resets should properly be hooked up to the GPC domains via the clocks
and resets DT properties.

For the clocks there is one complication: if the clocks are controlled
via BLK_CTL we can only enable them once the domain is powered up,
however the earlier designs using the GPCv2 assert resets as part of
the power up sequence, which needs the clocks to be running for the
reset to propagate. So depending on whether we have a power domain with
a BLK_CTL or not we need to enable the clocks before or after powering
up the domain. I guess we need a new DT property to specify which way
the domain needs to handled.

Regards,
Lucas



Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-26 Thread Adam Ford
On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa  wrote:
>
> On 20-10-25 11:05:32, Adam Ford wrote:
> > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut  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 0x3 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://www.spinics.net/lists/arm-kernel/msg843007.html
> 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.
>
I will go back and double check this now that we have both the
blt_crl->power-domain and the power-domain->blk_ctl.

> Will dig further, see what pops out.

I wasn't sure which direction to go with the name.  I can rename the
media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
imx8mp naming convention and the fact that it's controlling both the
display and the camera interface, however it's depending on the
dispmix GPC.

I'll submit a RFC V2 with the cross referencing to the GPC based on
Marek's Mini patch set, but we'll still have an issue where the Mini
and Nano have different syscon values to enable the clocks, and
Marek's branch has it card-coded, so my patch would effectively break
the Mini in order to make the Nano operate until we find a better
solution.

adam

>
> >
> >  video_pll1_ref_sel0002400
> >  0 0  5
> >video_pll1 000   59400
> > 0 0  5
> >   video_pll1_bypass   000   59400
> > 0 0  5
> >  video_pll1_out   000   59400
> > 0 0  5
> > disp_pixel000   59400
> > 0 0  5
> >lcdif_pixel_clk   000
> > 59400  0 0  5
> >disp_pixel_clk   000
> > 59400  0 0  5
> > dsi_phy_ref   0002700
> > 0 0  5
> >mipi_dsi_clkref   000
> > 2700  0 0  5
> >
> > I am not 100% certain the clock parents  in the clk block driver for
> > the 8MN are correct, and I am not seeing the mipi_dsi_pclk
> >
> > Once the dust settles on the GPC decision for Mini and Nano, I think
> > we'll need a more generic way to pass the bits we need to set in clock
> > block to the GPC.
> >
> > adam
> > >
> > > You can also apply the one for 8MM:
> > > 

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-26 Thread Abel Vesa
On 20-10-25 11:05:32, Adam Ford wrote:
> On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut  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 0x3 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://www.spinics.net/lists/arm-kernel/msg843007.html
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.

Will dig further, see what pops out.

> 
>  video_pll1_ref_sel0002400
>  0 0  5
>video_pll1 000   59400
> 0 0  5
>   video_pll1_bypass   000   59400
> 0 0  5
>  video_pll1_out   000   59400
> 0 0  5
> disp_pixel000   59400
> 0 0  5
>lcdif_pixel_clk   000
> 59400  0 0  5
>disp_pixel_clk   000
> 59400  0 0  5
> dsi_phy_ref   0002700
> 0 0  5
>mipi_dsi_clkref   000
> 2700  0 0  5
> 
> I am not 100% certain the clock parents  in the clk block driver for
> the 8MN are correct, and I am not seeing the mipi_dsi_pclk
> 
> Once the dust settles on the GPC decision for Mini and Nano, I think
> we'll need a more generic way to pass the bits we need to set in clock
> block to the GPC.
> 
> adam
> >
> > You can also apply the one for 8MM:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flinux-arm-kernel%2F20201003224555.163780-5-marex%40denx.de%2Fdata=04%7C01%7Cabel.vesa%40nxp.com%7Cae966cce11204214fb1908d878ffd492%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637392387462398200%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=M944BaOI7Sa1RmI0nwrshKaM7MGMEN5pWgjmYqXZkns%3Dreserved=0


Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-25 Thread Adam Ford
On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut  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 0x3 to 32e28008, the i.MX8MM can bring the display
clocks out of reset.

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.

 video_pll1_ref_sel0002400
 0 0  5
   video_pll1 000   59400
0 0  5
  video_pll1_bypass   000   59400
0 0  5
 video_pll1_out   000   59400
0 0  5
disp_pixel000   59400
0 0  5
   lcdif_pixel_clk   000
59400  0 0  5
   disp_pixel_clk   000
59400  0 0  5
dsi_phy_ref   0002700
0 0  5
   mipi_dsi_clkref   000
2700  0 0  5

I am not 100% certain the clock parents  in the clk block driver for
the 8MN are correct, and I am not seeing the mipi_dsi_pclk

Once the dust settles on the GPC decision for Mini and Nano, I think
we'll need a more generic way to pass the bits we need to set in clock
block to the GPC.

adam
>
> You can also apply the one for 8MM:
> https://lore.kernel.org/linux-arm-kernel/20201003224555.163780-5-ma...@denx.de/


Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-25 Thread Marek Vasut
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.

You can also apply the one for 8MM:
https://lore.kernel.org/linux-arm-kernel/20201003224555.163780-5-ma...@denx.de/


Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-25 Thread Abel Vesa
On 20-10-24 16:03:17, Adam Ford wrote:
> On Sat, Oct 24, 2020 at 3:23 PM Abel Vesa  wrote:
> >
> > On 20-10-24 11:20:12, Adam Ford wrote:
> > > There are some less-documented registers which control clocks and
> > > resets for the multimedia block which controls the LCDIF, ISI, MIPI
> > > CSI, and MIPI DSI.
> > >
> > > The i.Mx8M Nano appears to have a subset of the i.MX8MP registers with
> > > a couple shared registers with the i.MX8MM.  This series builds on the
> > > series that have been submitted for both of those other two platforms.
> > >
> > > This is an RFC because when enabling the corresponding DTS node, the
> > > system freezes on power on.  There are a couple of clocks that don't
> > > correspond to either the imx8mp nor the imx8mm, so I might have something
> > > wrong, and I was hoping for some constructive feedback in order to get
> > > the imx8m Nano to a similar point of the Mini and Plus.
> > >
> >
> > Thanks for the effort.
> 
> Sure thing!
> 
> >
> > I'm assuming this relies on the following patchset, right ?
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2020%2F10%2F24%2F139data=04%7C01%7Cabel.vesa%40nxp.com%7C7fdffcb44af4bbe808d8786044cf%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637391702150893977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=RIdR4O2Qx3q7ovfAO7O3kc%2BpXXMPzLQJWPoUAH305%2Bs%3Dreserved=0
> Abell,
> 
> Your link points right back to this e-mail.  ;-)

Sorry about that. Was kinda late here yesterday.

> 
> I based this work off:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg843906.htmldata=04%7C01%7Cabel.vesa%40nxp.com%7C7fdffcb44af4bbe808d8786044cf%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637391702150893977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=6B3UHZNULMB%2FtMzdWHckIOFlqFxEYwYdQclzqDtJ%2FNQ%3Dreserved=0
>   from Marek
> which I beleive is based on
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg836165.htmldata=04%7C01%7Cabel.vesa%40nxp.com%7C7fdffcb44af4bbe808d8786044cf%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637391702150893977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=I%2FfgIjzqwnjuSGOPfVKa%2BPbx950Be008sOon%2FDwSO1o%3Dreserved=0
>  from you.
> 
> I also have a GPC patch series located:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg847925.htmldata=04%7C01%7Cabel.vesa%40nxp.com%7C7fdffcb44af4bbe808d8786044cf%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637391702150893977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=dF%2Bgth7dwopoB2rvQ8IqspTZ7kWxvMYGS0dY%2F0gWgXE%3Dreserved=0
> 
> 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.

> adam
> 
> 
> >
> > > Adam Ford (3):
> > >   dt-bindings: clock: imx8mn: Add media blk_ctl clock IDs
> > >   dt-bindings: reset: imx8mn: Add media blk_ctl reset IDs
> > >   clk: imx: Add blk-ctl driver for i.MX8MN
> > >
> > >  drivers/clk/imx/clk-blk-ctl-imx8mn.c | 80 
> > >  include/dt-bindings/clock/imx8mn-clock.h | 11 
> > >  include/dt-bindings/reset/imx8mn-reset.h | 22 +++
> > >  3 files changed, 113 insertions(+)
> > >  create mode 100644 drivers/clk/imx/clk-blk-ctl-imx8mn.c
> > >  create mode 100644 include/dt-bindings/reset/imx8mn-reset.h
> > >
> > > --
> > > 2.25.1
> > >


Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-24 Thread Adam Ford
On Sat, Oct 24, 2020 at 3:23 PM Abel Vesa  wrote:
>
> On 20-10-24 11:20:12, Adam Ford wrote:
> > There are some less-documented registers which control clocks and
> > resets for the multimedia block which controls the LCDIF, ISI, MIPI
> > CSI, and MIPI DSI.
> >
> > The i.Mx8M Nano appears to have a subset of the i.MX8MP registers with
> > a couple shared registers with the i.MX8MM.  This series builds on the
> > series that have been submitted for both of those other two platforms.
> >
> > This is an RFC because when enabling the corresponding DTS node, the
> > system freezes on power on.  There are a couple of clocks that don't
> > correspond to either the imx8mp nor the imx8mm, so I might have something
> > wrong, and I was hoping for some constructive feedback in order to get
> > the imx8m Nano to a similar point of the Mini and Plus.
> >
>
> Thanks for the effort.

Sure thing!

>
> I'm assuming this relies on the following patchset, right ?
> https://lkml.org/lkml/2020/10/24/139
Abell,

Your link points right back to this e-mail.  ;-)

I based this work off:
https://www.spinics.net/lists/arm-kernel/msg843906.html  from Marek
which I beleive is based on
https://www.spinics.net/lists/arm-kernel/msg836165.html from you.

I also have a GPC patch series located:
https://www.spinics.net/lists/arm-kernel/msg847925.html

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.

adam


>
> > Adam Ford (3):
> >   dt-bindings: clock: imx8mn: Add media blk_ctl clock IDs
> >   dt-bindings: reset: imx8mn: Add media blk_ctl reset IDs
> >   clk: imx: Add blk-ctl driver for i.MX8MN
> >
> >  drivers/clk/imx/clk-blk-ctl-imx8mn.c | 80 
> >  include/dt-bindings/clock/imx8mn-clock.h | 11 
> >  include/dt-bindings/reset/imx8mn-reset.h | 22 +++
> >  3 files changed, 113 insertions(+)
> >  create mode 100644 drivers/clk/imx/clk-blk-ctl-imx8mn.c
> >  create mode 100644 include/dt-bindings/reset/imx8mn-reset.h
> >
> > --
> > 2.25.1
> >


Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

2020-10-24 Thread Abel Vesa
On 20-10-24 11:20:12, Adam Ford wrote:
> There are some less-documented registers which control clocks and 
> resets for the multimedia block which controls the LCDIF, ISI, MIPI 
> CSI, and MIPI DSI.
> 
> The i.Mx8M Nano appears to have a subset of the i.MX8MP registers with
> a couple shared registers with the i.MX8MM.  This series builds on the
> series that have been submitted for both of those other two platforms.
> 
> This is an RFC because when enabling the corresponding DTS node, the 
> system freezes on power on.  There are a couple of clocks that don't
> correspond to either the imx8mp nor the imx8mm, so I might have something
> wrong, and I was hoping for some constructive feedback in order to get
> the imx8m Nano to a similar point of the Mini and Plus.
> 

Thanks for the effort.

I'm assuming this relies on the following patchset, right ?
https://lkml.org/lkml/2020/10/24/139

> Adam Ford (3):
>   dt-bindings: clock: imx8mn: Add media blk_ctl clock IDs
>   dt-bindings: reset: imx8mn: Add media blk_ctl reset IDs
>   clk: imx: Add blk-ctl driver for i.MX8MN
> 
>  drivers/clk/imx/clk-blk-ctl-imx8mn.c | 80 
>  include/dt-bindings/clock/imx8mn-clock.h | 11 
>  include/dt-bindings/reset/imx8mn-reset.h | 22 +++
>  3 files changed, 113 insertions(+)
>  create mode 100644 drivers/clk/imx/clk-blk-ctl-imx8mn.c
>  create mode 100644 include/dt-bindings/reset/imx8mn-reset.h
> 
> -- 
> 2.25.1
>