RE: [PATCH 1/2] [media] V4L: atmel-isi: add code to enable/disableISI_MCK clock

2011-12-07 Thread Wu, Josh
Hi, Russell King

On Wed, Dec 07, 2011 at 4:50 PM, Russell King wrote:

 On Wed, Nov 30, 2011 at 06:06:43PM +0800, Josh Wu wrote:
 +/* Get ISI_MCK, provided by programmable clock or external clock
*/
 +isi-mck = clk_get(dev, isi_mck);
 +if (IS_ERR_OR_NULL(isi-mck)) {

 This should be IS_ERR()

So it means the clk_get() will never return NULL even when clk structure
is NULL in clk lookup entry. Right?

 +dev_err(dev, Failed to get isi_mck\n);
 +ret = isi-mck ? PTR_ERR(isi-mck) : -EINVAL;

   ret = PTR_ERR(isi-mck);

Best Regards,
Josh Wu
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] [media] V4L: atmel-isi: add code to enable/disableISI_MCK clock

2011-12-07 Thread Russell King - ARM Linux
On Wed, Dec 07, 2011 at 06:12:52PM +0800, Wu, Josh wrote:
 Hi, Russell King
 
 On Wed, Dec 07, 2011 at 4:50 PM, Russell King wrote:
 
  On Wed, Nov 30, 2011 at 06:06:43PM +0800, Josh Wu wrote:
  +  /* Get ISI_MCK, provided by programmable clock or external clock
 */
  +  isi-mck = clk_get(dev, isi_mck);
  +  if (IS_ERR_OR_NULL(isi-mck)) {
 
  This should be IS_ERR()
 
 So it means the clk_get() will never return NULL even when clk structure
 is NULL in clk lookup entry. Right?

It is not the drivers business to know whether NULL is valid or not.

clk_get() is defined to either return an error pointer, or a cookie
which the rest of the clk API must accept.

If an implementation decides that clk_get() can return NULL and deals
with that in the rest of the API (eg, to mean 'there is no clock but
don't fail for this') then drivers must not reject that.

If a driver rejects NULL then it is performing checks outside of the
definition of the clk API, and making assumptions about the nature of
valid cookies.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html