Re: ti_scm_syscon driver

On 07.09.2020 10:40, Oskar Holmlund via freebsd-arm wrote:
> Den måndag 7 september 2020 02:30:57 CEST, Thomas Skibo <[email protected]> skrev: 
> 
> Hello.
> 
> I am testing some code I have developed to provide the Zynq boards with
> a real clock driver and update the other device drivers to use the clock
> infrastructure.  It is also supposed to allow us to use "upstream" gnu
> dts files.  But, I ran into a problem booting a GENERIC kernel on my Zybo.
> The ti_scm_syscon driver, which has the "syscon" compatibility string,
> grabs the zynq-slcr node before my driver.  The compatibility property
> for the node in the GNU dts file is;
> 
>     compatible = "xlnx,zynq-slcr", "syscon", "simple-mfd";
> 
> I fixed the problem by having my probe routine return DEVICE_PROBE_SPECIFIC.
> But, the DEVICE_PROBE(9) man page seems to say I shouldn't do this.  Also, I
> see a lot of device nodes in other boards that have "syscon" along with their
> more specific driver name.  Aren't they going to run into problems too if
> the ti_scm_syscon driver matches them?  Should the ti_scm_syscon driver use
> a lower priority return value in its probe routine?
> 
> --Thomas Skibo
> 
> _______________________________________________
> [email protected] mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-arm
> To unsubscribe, send any mail to "[email protected]"
> 
> 
> Hi Thomas,
> 
> I'm sorry I've caused you trouble with ti_scm_syscon.c catchall for syscon.
> 
> I will try to point out the situation from TI dts.
> (head) sys/gnu/dts/arm/am33xx-l4.dtsi from line 270 the control module are defined and 286 the syscon part.
> The Control module are described here
> https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/omap/ctrl.txt
> the syscon part is not directly documented for TI its just there in the example.
> (of course this exist https://www.kernel.org/doc/Documentation/devicetree/bindings/mfd/syscon.txt )
> 
> I'm glad for any suggestions to solve the issue to not interfere with other drivers.
> I can test your proposal to lower the priority.
> Or get TI to change syscon to something more specific in the DTS?
> Maybe its better if i hardcode ti_scm.c to add the syscon child and remove the probe part from ti_scm_syscon.c?
> Other suggestions?
>

I completely overlooked this problem in review, sorry for that.
I think that we should delete ti_scm_syscon driver at all - it doesn't
contain a single bit of TI specific code and it incorrectly steals pure
"syscon" compatibility string.
Instead, we should expand existing existing extres/syscon_generic driver
and teach it about „simple-bus" compatibility string. Exactly as we
already do in fdt/simple_mfd driver.
Thomas, priority returned by bus_probe method works only for drivers
probed at same bus_pass (in other word, bus_pass have precedence over
priority returned by bus_probe() method. That’s also mean that more
specific syscon driver must be probed at BUS_PASS_DEFAULT bus pass or
earlier.

Both fact (priority interference with bus pass and very late syscon
instantiation - syscon should be one of first drivers available) looks
like bugs for me...

I will try to fix existing syscon_generic ASAP.
Michal

_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[email protected]"