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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to