Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm

[Leonard Crestez <leonard.crestez@nxp.com>]

On 10.10.2019 17:43, Angus Ainslie wrote:

>>>>> This adds a platform driver for the i.MX8MM SoC.
>>>>>
>>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>>>>> ---
>>>>>    drivers/interconnect/imx/Kconfig          |   4 +
>>>>>    drivers/interconnect/imx/Makefile         |   1 +
>>>>>    drivers/interconnect/imx/imx8mm.c         | 114
>>>>> ++++++++++++++++++++++
>>>>>    include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>>>>>    4 files changed, 168 insertions(+)
>>>>>    create mode 100644 drivers/interconnect/imx/imx8mm.c
>>>>>    create mode 100644 include/dt-bindings/interconnect/imx8mm.h
>>>>
>>>> Do you plan to add such a driver for imx8mq too?
>>>
>>> Yes! The topology is different (serving different IP blocks) but no
>>> functional code changes are required between 8mm 8mn 8mq.
>>
>> Thanks for the update, that's good to hear. I'll get back to you when I
>> come around to test this and wish you good progress until then :)
>>
> I've taken up this work while Martin is on leave.
> 
> I've integrated your u-boot and ATF on our board and I have a couple of
> questions. Our board is running imx8mq B0 (Rev 2.0) silicon.
> 
> It looks like this line limits the training frequencies to 800 MHz and
> 166 MHz

Yes! This is due to a hardware errata which was solved in B1: DRAM pll 
can't be disabled. This means that instead of 25/100/800 freqs are 
166/800, and this requires code changes.

> Does 100 MHz and 25 MHz not work on B0 ?

No, lower rates require dram clk from a composite slice (dram_alt_root)

> I added the ddrc_and noc opp as well as the 166MHz opp
> 
> I also added the interconnects ( do we need them on imx8mq ? )

The interconnect stuff is not required to switch dram frequency, it's 
for device to make minimum bandwidth requests. It an additional feature 
on top.

As a hack I configured FEC to do so but a saner example would be to 
request bandwidth based on display resolution and color depth.

> I had to add a hack as the PM QoS was limiting the bus speed to 399MHz ,
> if you have any ideas why that would be appreciated.

You probably need to set ethernet down (which is awkward) or better just 
drop the interconnect properties and test using the devfreq userspace 
governor.

> The driver is probing
> 
> [   12.136537] bus: 'platform': driver_probe_device: matched device
> 3d400000.dram-controller with driver imx-ddrc-devfrq
> [   12.147259] bus: 'platform': really_probe: probing driver
> imx-ddrc-devfreq with device 3d400000.dram-controller
> [   12.157382] imx-ddrc-devfreq 3d400000.dram-controller: no pinctrl
> handle
> [   12.164197] arm_smcc rate 0 800000000
> [   12.167880] arm_smcc rate 1 166750000
> [   12.171778] of: _opp_add_static_v2: turbo:0 rate:25000000 uv:0
> uvmin:0 uvmax:0 latency:0
> [   12.179994] of: _opp_add_static_v2: turbo:0 rate:100000000 uv:0
> uvmin:0 uvmax:0 latency:0
> [   12.188311] of: _opp_add_static_v2: turbo:0 rate:166750000 uv:0
> uvmin:0 uvmax:0 latency:0
> [   12.196606] of: _opp_add_static_v2: turbo:0 rate:800000000 uv:0
> uvmin:0 uvmax:0 latency:0
> [   12.204930] imx-ddrc-devfreq 3d400000.dram-controller: events from
> pmu imx8_ddr0
> [   12.212403] Added freq 0 25000000
> [   12.215742] Added freq 1 100000000
> [   12.219177] Added freq 2 166750000
> [   12.222648] Added freq 3 800000000
> [   12.226105] device: 'devfreq0': device_add
> [   12.230287] PM: Adding info for No Bus:devfreq0
> [   12.234864] driver: 'imx-ddrc-devfreq': driver_bound: bound to device
> '3d400000.dram-controller'
> [   12.243699] bus: 'platform': really_probe: bound device
> 3d400000.dram-controller to driver imx-ddrc-devfreq
> 
> Add seems to run correctly until it tries to adjust the clock to 166MHz
> 
> [   19.555482] ddrc checking rate 800000000 166750000
> [   19.555489] ddrc checking rate 166750000 166750000
> [   19.560442] bus: 'usb-serial': really_probe: bound device ttyUSB0 to
> driver option1
> [   19.568751] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about to
> change freq 800000000 to 166750000
> 
> And the board hangs there. Any ideas on how to get past this ?

Please try this ATF patch:

https://github.com/cdleonard/arm-trusted-firmware/commit/783fc2b2c4266bfdb5218e4d9b6b2bc90849e0e9

I tested switching on imx8mq-evk with B0 SoC but a few additional 
changes are required in kernel to support switching between rates which 
are both backed by PLL:

* Mark the PLL CLK_GET_RATE_NOCACHE
* Set the rate to 166935483 exactly (to match clk_get_rate)
* Make the rounding in imx-ddrc more generous.

I will integrate these changes into the next version.

--
Regards,
Leonard