RE: [PATCH 2/2] usb: chipidea: usbmisc: set over current polarity for imx6 and imx7

[Jun Li <jun.li@xxxxxxx>]

Hi Peter,

> -----Original Message-----
> From: Peter Chen [mailto:hzpeterchen@xxxxxxxxx]
> Sent: Friday, July 15, 2016 5:21 PM
> To: Jun Li <jun.li@xxxxxxx>
> Cc: Peter Chen <peter.chen@xxxxxxx>; linux-usb@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 2/2] usb: chipidea: usbmisc: set over current polarity
> for imx6 and imx7
> 
> On Fri, Jul 15, 2016 at 07:38:23AM +0000, Jun Li wrote:
> > Hi,
> > > -----Original Message-----
> > > From: Peter Chen [mailto:hzpeterchen@xxxxxxxxx]
> > > Sent: Friday, July 15, 2016 3:02 PM
> > > To: Jun Li <jun.li@xxxxxxx>
> > > Cc: Peter Chen <peter.chen@xxxxxxx>; linux-usb@xxxxxxxxxxxxxxx
> > > Subject: Re: [PATCH 2/2] usb: chipidea: usbmisc: set over current
> > > polarity for imx6 and imx7
> > >
> > > On Tue, Jul 12, 2016 at 03:24:49PM +0800, Li Jun wrote:
> > > > As all usb power supply use low active for over current flag on
> > > > imx6
> > > > imx7 boards, and the default register setting(0) is for high
> > > > active, this patch is to correct it.
> > > >
> > >
> > > We may can't ensure all USB power switch chips work like that, I
> > > suggest you making this as default.
> > >
> > > I will change the commit log like below if you are ok.
> > >
> > > As most of all usb power switch chips use active-low for over
> > > current flag, but the default register setting(0) is for active-high
> > > at imx6/imx7, this patch changes default value as active-low.
> >
> > Looks better, I am okay with it except a tiny comment :%s/As most of
> > all usb power/As most of usb power
> >
> > Li Jun
> > >
> Since we can't break current default behaviour, but with your patch, the
> imx6sx sdb board creates over current event.
> 
> I think you may need to introduce a flag for OC polarity, and use it for
> exist platforms if necessary. It can narrow down affect only on single
> platform.
> 
> For new platforms, you can change SoC values by default.

To keep those platforms without specify either disable-over-current
or over-current-polarity out of problem, we need set the disable_oc
to be "1", or oc_polarity to be "1"(then register value to be "0"
as before) for them.

if (!of_property_read_u32(np, "over-current-polarity", &ret))
        data->oc_polarity = ret ? 1 : 0;
else
        data->oc_polarity = 1;

or

if (!of_property_read_u32(np, "over-current-polarity", &ret))
        data->oc_polarity = ret ? 1 : 0;
else
        data->disable_oc = 1;

which way do you prefer?

Li Jun    
> 
> --
> 
> 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