> -----Original Message----- > From: Masahiro Yamada [mailto:yamada.masah...@socionext.com] > Sent: 2016年8月22日 0:54 > To: Stephen Warren <swar...@wwwdotorg.org> > Cc: Wenyou Yang - A41535 <wenyou.y...@microchip.com>; Wenyou Yang - > A41535 <wenyou.y...@microchip.com>; U-Boot Mailing List <u- > b...@lists.denx.de>; Stephen Warren <swar...@nvidia.com>; Michal Simek > <michal.si...@xilinx.com> > Subject: Re: [U-Boot] [PATCH v1] clk: clk-uclass: Check ops pointer before > use it > > 2016-08-19 12:52 GMT+09:00 Stephen Warren <swar...@wwwdotorg.org>: > > On 08/18/2016 07:49 PM, wenyou.y...@microchip.com wrote: > >> > >> Add Simon and Andreas in loop > >> > >>> -----Original Message----- > >>> From: Stephen Warren [mailto:swar...@wwwdotorg.org] > >>> Sent: 2016年8月18日 11:56 > >>> To: Wenyou Yang - A41535 <wenyou.y...@microchip.com>; > >>> wenyou.y...@atmel.com > >>> Cc: u-boot@lists.denx.de; swar...@nvidia.com; > >>> michal.si...@xilinx.com > >>> Subject: Re: [U-Boot] [PATCH v1] clk: clk-uclass: Check ops pointer > >>> before use it > >>> > >>> On 08/17/2016 09:53 PM, wenyou.y...@microchip.com wrote: > >>>> > >>>> Hi Stephen, > >>>> > >>>>> -----Original Message----- > >>>>> From: Stephen Warren [mailto:swar...@wwwdotorg.org] > >>>>> Sent: 2016年8月18日 11:46 > >>>>> To: Wenyou Yang - A41535 <wenyou.y...@microchip.com>; > >>>>> wenyou.y...@atmel.com > >>>>> Cc: u-boot@lists.denx.de; swar...@nvidia.com; > >>>>> michal.si...@xilinx.com > >>>>> Subject: Re: [U-Boot] [PATCH v1] clk: clk-uclass: Check ops > >>>>> pointer before use it > >>>>> > >>>>> On 08/17/2016 06:30 PM, wenyou.y...@microchip.com wrote: > >>>>>> > >>>>>> HI Stephen, > >>>>>> > >>>>>>> -----Original Message----- > >>>>>>> From: Stephen Warren [mailto:swar...@wwwdotorg.org] > >>>>>>> Sent: 2016年8月17日 23:59 > >>>>>>> To: Wenyou Yang <wenyou.y...@atmel.com> > >>>>>>> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stephen Warren > >>>>>>> <swar...@nvidia.com>; Michal Simek <michal.si...@xilinx.com> > >>>>>>> Subject: Re: [U-Boot] [PATCH v1] clk: clk-uclass: Check ops > >>>>>>> pointer before use it > >>>>>>> > >>>>>>> On 08/17/2016 01:05 AM, Wenyou Yang wrote: > >>>>>>>> > >>>>>>>> Add check ops pointer before use it. Otherwise, it will cause > >>>>>>>> the runtime error for the clk devices without ops callback. > >>>>>>> > >>>>>>> > >>>>>>> Other uclasses like reset, power domain, and mailbox don't do this. > >>>>>>> All drivers must have an ops pointer, or they can't be useful. > >>>>>>> I'm not sure this patch is necessary. Is it just a debugging aid > >>>>>>> so if the driver writer forgets to set the ops pointer the > >>>>>>> system will error out rather than crashing? If so, a post-bind > >>>>>>> hook in the uclass that > >>>>> > >>>>> refuses the driver if it hasn't set the ops pointer would be better. > >>>>>> > >>>>>> > >>>>>> Sorry, I don't agree with you. > >>>>>> > >>>>>> Not all drivers have an ops pointer. > >>>>>> > >>>>>> If you grep U_BOOT_DRIVER , you will find that there are some > >>>>>> drivers without > >>>>> > >>>>> an ops pointer. > >>>>>> > >>>>>> > >>>>>> We should not suppose a driver should have something, I think. > >>>>> > >>>>> > >>>>> But without an ops pointer, the driver can do nothing and provide > >>>>> no services. > >>>>> Why is that useful? > >>>> > >>>> > >>>> There are some nodes without compatible in device tree, as a child > >>>> of some node, for example, pinctrl child node or for my code > >>>> peripheral clock node. > >>>> > >>>> These nodes also need to be bound before using them. They require > >>>> such driver > >>> > >>> to bind them. > >>> > >>> That seems unrelated. A node without a compatible value won't > >>> instantiate a device object, and hence needs no driver. > >> > >> > >> But there are such nodes and drivers (i.e., U_BOOT_DRIVER) existed in > >> U-Boot, as I said before. > >> > >> Is it harmful to add this check? > >> > >> I know, if not, it will produce a wild pointer, such as NULL->of_xlate. > > > > > > I see this in drivers/clk/at91/{pmc,sckc}.c. The drivers in those files > > exist solely to cause DT sub-nodes to be enumerated, and aren't clock > > providers themselves. I believe the fix is to change them from UCLASS_CLK to > > UCLASS_SOMETHING_ELSE, e.g. UCLASS_MISC. ops are already optional > for that > > uclass. > > Right. > drivers/clk/at91/{pmc,sckc}.c are doing wrong. > > I think UCLASS_SIMPLE_BUS is the best > because you do not have to bind child nodes explicitly. > simple_bus_post_bind() will do it for you.
Thank you for your advice, I will try it. > > > > -- > Best Regards > Masahiro Yamada Best Regards, Wenyou Yang _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
