Page MenuHomeFreeBSD

Ti AM335x move to dev/extres/clk framework
ClosedPublic

Authored by oskar.holmlund_ohdata.se on Jun 3 2020, 4:06 PM.

Details

Summary

I'm aware its a "big" patch, but i cant see how to make it into smaller steps.

Tested on BBB and PB.

  • HDMI / LCD
  • GPIO
  • ADC
  • PWM
  • I2C
  • SPI
  • USB keyboard / webcam

Not working
PRU - need DTS from Linux5.7 + glue.

OMAP4 will probably not work at current state.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Only had a quick look.
Could you put the dev/extres/clk changes in another review and add me and mmel@ please ?
For the dts changes, could you explain why it's needed and also add comments to show that it's a FreeBSD specific change ?

OMAP4 will probably not work at current state.

So is the breakage big or small? I ask because I have a pandaboard which is omap4 iirc.

In D25118#553388, @manu wrote:

Only had a quick look.
Could you put the dev/extres/clk changes in another review and add me and mmel@ please ?

I fix it tomorrow.

For the dts changes, could you explain why it's needed and also add comments to show that it's a FreeBSD specific change ?

DTS changes in sys/gnu/dts/arm/am33xx.dtsi are bugs, the ranges are to small compared to what is stated in TRM - i will send patch to lkml.

In am33xx-l4.dtsi its not really a "bug" but i cant see why anyone wants a timer with a clkctrl (in parent sysc) but no functional clock.
Another place they with similar problem is on the USB, i cant find any reference to the usbotg_fck@47c so i did a hack:ish solution and that will probably be changed in the future.

In D25118#553391, @imp wrote:

OMAP4 will probably not work at current state.

So is the breakage big or small? I ask because I have a pandaboard which is omap4 iirc.

Small, its in arm/ti/usb/omap_* and arm/ti/omap4/omap4_prcm_clks.c. I will try to fix it aswell.

I've just commited the DTS changes (after updating to 5.7).

So, I will not have time to properly review this before next week but I suggest that you do the following :

  • Move the dev/extres/clk changes into another review (as said before)
  • Rebase your work since now the dts changes are in
  • Move clk drivers under a clk/ subdir in sys/arm/ti (this is was we have in most of our port so it seems cleaner do it that way)

I think that if we really wanted to we could do a lot of small patches that slowly migrate the TI code to the clk framework but TBH that seems a waste of time.
We know that if a problem appears and we need to bisect we will end up finding that the clock move is the culprit.
I guess it's easier for everyone if we do a big switch for everything.

In D25118#554295, @manu wrote:

I've just commited the DTS changes (after updating to 5.7).

Thank you, I have sent it to linux-omap mailinglist
https://marc.info/?l=linux-omap&r=1&b=202006&w=2

  • Move clock related files to subfolder /sys/arm/ti/clk
  • extract sys/dev/extres/clk to another review
  • due to recent update of devicetree @ head the dts are removed

I appreciate suggestions on how to make the startup more efficient. First it finds the clocks below prcm@0 and much later the necessary clock references under scm_conf@0. All devices are probed inbetween ("ti,sysc" reference the ti,clkctrls defined in scm_conf@0) and fails due to the clock is not ready.

Todo: sys/arm/ti/am335x/am335x_scm.c driver used for internal temperature are temporarly removed - I need rewrite it to use syscon interface.

Update sys/arm/ti/am335x/am335x_scm.c to use syscon interface for reading and writing to registers in that memory range of the control module (TRM chapter 9).

Create a "virtual clock" (not mentioned in the dts) for gdbclk functional clocks for GPIO1-4 and get rid of changes in dev/extres/clk/clk.c (D25175) needed in earlier revision.

I think I'm okay with this now, @mmel what do you think ?

Fix sys/arm/ti/ti_pruss.c to activate pruss subsystems
Add shim driver ti_prm to reset the pruss.

PRU part tested on PB techlab PRU generated PWM to buzzer + blinking usr3:ish LED on board PB
The PRU needs a DTS overlay to be enabled. Probably out of scope for this patchset?

Retested the patch as follow:
HDMI - works with HP envy 24
LCD interface - works with newhaven nhd-7.0ctp-cape-L
PWM - Backlight on newhaven nhd7.../ RGB led on PocketBeagle techlab
USB - General keyboard from hp
SPI - mcp23 io expander Pocketbeagle techlab
I2C - tps/nxp tda19988 (BBBlack), NXP MMA8453q accelerometer on PB techlab
CPSW - Ethernet on BBBlack
Analog - PB Techlab light dependent resistor
GPIO - USR led on PB/BBBlack

i have ordered a pandaboard es. Changes related to omap4 will come later.

It also looks OK for me. I have a few small items that we should rethink or fix, but we can do all of these (including pandaboard support) as follow-up fixes. I can commit this as is over the weekend and then we can start discussing improvements (and I also hope to have a little more free time from next week).
Oscar, manu do you agree?
Problematic (for me) items are:

  • The ti_dpll_clk_set_freq routine should respect rounding (dpll) and DRY_RUN.
  • I’m not a big friend of bulk clock enable in ti_sysc_clock_enable().
  • probably , it will be easier to convert scm_clocks and prcm_clocks instances from FDT data driven to compiled-in table driven clocks implementation.

But all these are minor and should be discussed firstly...

In D25118#558687, @mmel wrote:

It also looks OK for me. I have a few small items that we should rethink or fix, but we can do all of these (including pandaboard support) as follow-up fixes. I can commit this as is over the weekend and then we can start discussing improvements (and I also hope to have a little more free time from next week).
Oscar, manu do you agree?
Problematic (for me) items are:

  • The ti_dpll_clk_set_freq routine should respect rounding (dpll) and DRY_RUN.
  • I’m not a big friend of bulk clock enable in ti_sysc_clock_enable().
  • probably , it will be easier to convert scm_clocks and prcm_clocks instances from FDT data driven to compiled-in table driven clocks implementation.

But all these are minor and should be discussed firstly...

Sounds good to me.

This revision is now accepted and ready to land.Jun 19 2020, 11:23 AM
In D25118#558687, @mmel wrote:

It also looks OK for me. I have a few small items that we should rethink or fix, but we can do all of these (including pandaboard support) as follow-up fixes. I can commit this as is over the weekend and then we can start discussing improvements (and I also hope to have a little more free time from next week).
Oscar, manu do you agree?
Problematic (for me) items are:

  • The ti_dpll_clk_set_freq routine should respect rounding (dpll) and DRY_RUN.
  • I’m not a big friend of bulk clock enable in ti_sysc_clock_enable().
  • probably , it will be easier to convert scm_clocks and prcm_clocks instances from FDT data driven to compiled-in table driven clocks implementation.

But all these are minor and should be discussed firstly...

Fine with me for the moment. I have a couple of "Fixme" in the code to bring up to discuss/work on.

This revision was automatically updated to reflect the committed changes.