On Fri, 16 Feb 2024 at 17:33, Kathiravan Thirumoorthy <quic_kathirav@xxxxxxxxxxx> wrote: > > > > On 2/14/2024 8:14 PM, Andrew Lunn wrote: > > On Wed, Feb 14, 2024 at 02:49:41PM +0530, Kathiravan Thirumoorthy wrote: > >> > >> > >> On 1/26/2024 1:35 AM, Andrew Lunn wrote: > >>> On Mon, Jan 22, 2024 at 11:26:58AM +0530, Kathiravan Thirumoorthy wrote: > >>>> gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk, gcc_nssnoc_nsscc_clk are > >>>> enabled by default and it's RCG is properly configured by bootloader. > >>> > >>> Which bootloader? Mainline barebox? > >> > >> > >> Thanks for taking time to review the patches. I couldn't get time to respond > >> back, sorry for the delay. > >> > >> I was referring to the U-boot which is delivered as part of the QSDK. I will > >> call it out explicitly in the next patch. > > > > I've never used QSDK u-boot, so i can only make comments based on my > > experience with other vendors build of u-boot. That experience is, its > > broken for my use cases, and i try to replace it as soon as possible > > with upstream. > > > > I generally want to TFTP boot the kernel and the DT blob. Sometimes > > vendor u-boot has networking disabled. Or the TFTP client is > > missing. If it is there, the IP addresses are fixed, and i don't want > > to modify my network to make it compatible with the vendor > > requirements. If the IP addresses can be configured, sometimes there > > is no FLASH support so its not possible to actually write the > > configuration to FLASH so that it does the right thing on reboot > > etc... > > > > Often the vendor u-boot is a black box, no sources. Can you give me a > > git URL for the u-boot in QSDK? If the sources are open, i could at > > least rebuild it with everything turned on. > > > You can get the source at > https://git.codelinaro.org/clo/qsdk/oss/boot/u-boot-2016/-/tree/NHSS.QSDK.12.2?ref_type=heads > > You should be able to TFTP the images, write into the flash and > configure the IP and so on... > > > > > > But still, it is better that Linux makes no assumptions about what the > > boot loader has done. That makes it much easier to change the > > bootloader. > > > >>>> Some of the NSS clocks needs these clocks to be enabled. To avoid > >>>> these clocks being disabled by clock framework, drop these entries > >>>> from the clock table and enable it in the driver probe itself. > >>> > >>> If they are critical clocks, i would expect a device to reference > >>> them. The CCF only disabled unused clocks in late_initcall_sync(), > >>> which means all drivers should of probed and taken a reference on any > >>> clocks they require. > >> > >> > >> Some of the NSSCC clocks are enabled by bootloaders and CCF disables the > >> same (because currently there are no consumers for these clocks available in > >> the tree. These clocks are consumed by the Networking drivers which are > >> being upstreamed). > > > > If there is no network drivers, you don't need clocks to the > > networking hardware. So CCF turning them off seems correct. > > > Yeah agree with your comments. > > QSDK's u-boot enables the network support, so the required NSSCC clocks > are turned ON and left it in ON state. CCF tries to disables the unused > NSSCC clocks but system goes for reboot. > > > Reason being, to access the NSSCC clocks, these GCC clocks > (gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk, gcc_nssnoc_nsscc_clk) > should be turned ON. But CCF disables these clocks as well due to the > lack of consumer. This means that NSSCC is also a consumer of those clocks. Please fix both DT and nsscc driver to handle NSSNOC clocks. > > Once you have actual drivers, this should solve itself, the drivers > > will consume the clocks. > > > Given that, NSSCC is being built as module, there is no issue in booting > the kernel. But if you do insmod of the nsscc-ipq5332.ko, system will > reset. > > Without the networking drivers, there is no need to install this module. > And as you stated, once the drivers are available, there will be no issues. > > So can I explain the shortcomings of installing this module without the > networking drivers in cover letter and drop this patch all together? No. Using allyesconfig or allmodconfig and installing the full modules set should work. > >> However looking back, gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk, > >> gcc_nssnoc_nsscc_clk are consumed by the networking drivers only. So is it > >> okay to drop these clocks from the GCC driver and add it back once the > >> actual consumer needs it? > > > > But why should you remove them. If nothing is using them, they should > > be turned off. > > > > Andrew > -- With best wishes Dmitry
