Re: [PATCH v2 2/7] usb: chipidea: add otg file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 20, 2012 at 02:43:40PM +0100, Marc Kleine-Budde wrote:
> On 11/20/2012 02:40 PM, Peter Chen wrote:
> > On Tue, Nov 20, 2012 at 12:41:03PM +0100, Marc Kleine-Budde wrote:
> >> On 11/20/2012 02:33 AM, Peter Chen wrote:
> >>> On Mon, Nov 19, 2012 at 05:05:16PM +0100, Marc Kleine-Budde wrote:
> >>>>> +int ci_hdrc_otg_init(struct ci13xxx *ci)
> >>>>> +{
> >>>>> +	/* Useless at current */
> >>>>> +	ci->otg.set_peripheral = ci_otg_set_peripheral;
> >>>>> +	ci->otg.set_host = ci_otg_set_host;
> >>>>> +	if (!IS_ERR_OR_NULL(ci->transceiver))
> >>>>              ^^^^^^^^^^^^^^
> >>>>
> >>>> Is considered harmful by some people. Can we test against NULL here?
> >>>
> >>> Any stories that it is harmful?
> >>
> >> Your use-case isn't but there others are:
> >> http://www.mail-archive.com/linux-omap@xxxxxxxxxxxxxxx/msg78030.html
> > 
> > Thanks, marc. It said below code is error if the foo is really null.
> > if (IS_ERR_OR_NULL(foo))
> > 	return PTR_ERR(foo);
> 
> Yes. So my next comment is nitpicking: :)
> 
> > For this case, the ci->transceiver may be null or errno pointer, it
> > needs IS_ERR_OR_NULL.
> 
> Yes, but you can set ci->transceiver to NULL in case of an error when
> assigning it, to avoid IS_ERR_OR_NULL here.
OK, as the chipidea/udc.c also uses this macro. I will submit a patch
to change them together later, not sure if it is necessary for everyone.

> 
> Marc
> 
> -- 
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
> 



-- 

Best Regards,
Peter Chen

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux