OMAP2+ hwmod fixes
Get series with b4
b4 am [email protected]Versions newest first
- v1 Latest2 of 3 patches 0 repliesNew
2 of 3 patches are in this archive. The cover letter announces 3.
Patches in v1 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/3 | OMAP2+: hwmod: Avoid setup if clock lookup failed | +0 | 0 replies | |
| 2/3 | OMAP2+: hwmod: Fix what _init_clock returns | +0 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [PATCH 0/3] OMAP2+ hwmod fixes
Hi Rajendra,
Quoted 32 lines · Rajendra
Hi Benoit,
Quoted 5 lines
-----Original Message-----
From: Cousson, Benoit [mailto:b-cousson at ti.com]
Sent: Wednesday, February 16, 2011 6:37 PM
To: Nayak, Rajendra
Cc: linux-omap at vger.kernel.org; paul at pwsan.com;linux-arm-kernel at lists.infradead.org
Quoted 11 lines
Subject: Re: [PATCH 0/3] OMAP2+ hwmod fixes
Hi Rajendra,
Quoted 5 lines · Rajendra
This series fixes some hwmod api return values and also adds some state checks.
The hwmod iterator functions are made to continue and not break if one of the callback functions ends up with an error.Quoted 3 lines
By doing that, you change the behavior of this function. I'm not sure I fully understand why.
Could you elaborate on the use case?Since these functions iterate over all hwmods calling a common callback function, there might be cases wherein the callback function for *some* hwmods might fail. For instance, if you run through all hwmods and try to clear the context registers for all of them, for some hwmods which might not have context registers the callback function might return a
-EINVAL, however that should not stop you from attempting to clear the context
Quoted 8 lines
registers for the rest of the hwmods which have them and abort the function midway, no? This is more hypothetical, however the real usecase that prompted me with this patch was when I had some wrong state check in
_setupfunction, and the iterator would stop with the first failure and not even attempt to setup the rest of the hwmods.
Yeah, but by using that function you implicitly accept that an error will break the loop, so the function you pass to the iterator should be written for that. Meaning if you do not want to exit on error you should not report an error.
Quoted 7 lines
Quoted 6 lines
To avoid that behavior in the past, I was just returning 0 in case of failure to avoid stopping the iteration. It looks like you do not want to stop the iteration but still retrieve the error.
I do not see in this series what you plan to do with the error at the end of the iteration.
Quoted 5 lines
Most users of these iterators would just use the non-zero return value to throw an error/warning out stating there were failures running through all the callback functions. That does not change with this patch, and they can still do it.
Except that now, the iterator is not able anymore to stop on error if needed potentially.
My point is that you are changing the behavior of this function without maintaining the legacy.
So maybe creating a new iterator is a better approach.
Even is this feature is not used today I have some secret plan for this behavior in the near future :-)
But my initial comment is still valid, if you do not care about the final error status after the iteration, you'd better not return any error at the beginning.
This was the behavior or the _setup until now.
Regards,
Benoit