Details

Reviewers

manu
mmel

Commits

rS363700: Move Ti AM335x to dev/extres/clk framework.

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

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

oskar.holmlund_ohdata.se created this revision.Jun 3 2020, 4:06 PM

Herald added 1 blocking reviewer(s): manu. · View Herald TranscriptJun 3 2020, 4:06 PM

Herald added subscribers: loos, andrew, imp. · View Herald Transcript

oskar.holmlund_ohdata.se requested review of this revision.Jun 3 2020, 4:06 PM

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.

mmel added a subscriber: mmel.Jun 4 2020, 4:28 AM

manu mentioned this in rS361849: dts: patch the am33xx dts for upcoming clock support.Jun 5 2020, 8:15 PM

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.

oskar.holmlund_ohdata.se mentioned this in D25175: Add functions to sys/dev/extres/clk.Jun 7 2020, 10:56 AM

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.

Closed by commit rS363700: Move Ti AM335x to dev/extres/clk framework. (authored by mmel). · Explain WhyJul 30 2020, 2:45 PM

This revision was automatically updated to reflect the committed changes.

mmel added a commit: rS363700: Move Ti AM335x to dev/extres/clk framework..

Ti AM335x move to dev/extres/clk frameworkClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 72791

sys/arm/ti/am335x/am3359_cppi41.c

sys/arm/ti/am335x/am335x_dmtimer.c

sys/arm/ti/am335x/am335x_dmtpps.c

sys/arm/ti/am335x/am335x_dmtreg.h

sys/arm/ti/am335x/am335x_gpio.c

sys/arm/ti/am335x/am335x_lcd.c

sys/arm/ti/am335x/am335x_musb.c

sys/arm/ti/am335x/am335x_prcm.c

sys/arm/ti/am335x/am335x_pwmss.c

sys/arm/ti/am335x/am335x_rtc.c

sys/arm/ti/am335x/am335x_scm.c

sys/arm/ti/am335x/am335x_usb_phy.c

sys/arm/ti/am335x/am335x_usbss.c

sys/arm/ti/am335x/files.am335x

sys/arm/ti/clk/clock_common.h

sys/arm/ti/clk/clock_common.c

sys/arm/ti/clk/ti_clk_clkctrl.h

sys/arm/ti/clk/ti_clk_clkctrl.c

sys/arm/ti/clk/ti_clk_dpll.h

sys/arm/ti/clk/ti_clk_dpll.c

sys/arm/ti/clk/ti_clkctrl.c

sys/arm/ti/clk/ti_divider_clock.c

sys/arm/ti/clk/ti_dpll_clock.c

sys/arm/ti/clk/ti_gate_clock.c

sys/arm/ti/clk/ti_mux_clock.c

sys/arm/ti/cpsw/if_cpsw.c

sys/arm/ti/files.ti

sys/arm/ti/omap4/files.omap4

sys/arm/ti/ti_adc.c

sys/arm/ti/ti_edma3.c

sys/arm/ti/ti_gpio.c

sys/arm/ti/ti_hwmods.h

sys/arm/ti/ti_hwmods.c

sys/arm/ti/ti_i2c.c

sys/arm/ti/ti_mbox.c

sys/arm/ti/ti_omap4_cm.h

sys/arm/ti/ti_omap4_cm.c

sys/arm/ti/ti_pinmux.c

sys/arm/ti/ti_prcm.h

sys/arm/ti/ti_prcm.c

sys/arm/ti/ti_pruss.c

sys/arm/ti/ti_scm.c

sys/arm/ti/ti_scm_syscon.c

sys/arm/ti/ti_sdhci.c

sys/arm/ti/ti_sdma.c

sys/arm/ti/ti_spi.c

sys/arm/ti/ti_sysc.h

sys/arm/ti/ti_sysc.c

sys/arm/ti/ti_wdt.c

sys/arm/ti/usb/omap_ehci.c

sys/arm/ti/usb/omap_host.c

sys/arm/ti/usb/omap_tll.c

sys/dev/uart/uart_dev_ti8250.c

Ti AM335x move to dev/extres/clk framework
ClosedPublic
Actions

Revision Contents
Changeset List