RE: [PATCH v4 01/39] ARM: OMAP2+: gpmc: driver conversion

["Mohammed, Afzal" <afzal@xxxxxx>]

Hi Jon,

On Mon, May 07, 2012 at 21:32:58, Hunter, Jon wrote:

> >>> +	/* no waitpin */
> >>> +	case 0:
> >>> +		break;
> >>> +	default:
> >>> +		dev_err(gpmc->dev, "multiple waitpins selected on CS:%u\n", cs);
> >>> +		return -EINVAL;
> >>> +		break;
> >>> +	}
> >>
> >> Why not combined case 0 and default? Both are invalid configurations so
> >> just report invalid selection.
> > 
> > Case 0 is not invalid, a case where waitpin is not used, default refers
> > to when a user selects multiple waitpins wrongly.
> 
> Ok. Then for case 0, just return here. If the polarity is set, you could
> print an error here.

Different ways of doing things, this looks cleaner to me as already it is
checked, and time of execution in both cases would not differ much.

> >>> +		if (gd->have_waitpin) {
> >>> +			if (gd->waitpin != idx ||
> >>> +					gd->waitpin_polarity != polarity) {
> >>> +				dev_err(gpmc->dev, "error: conflict: waitpin %u with polarity %d on device %s.%d\n",
> >>> +					gd->waitpin, gd->waitpin_polarity,
> >>> +					gd->name, gd->id);
> >>> +				return -EBUSY;
> >>> +			}
> >>> +		} else {
> >>
> >> Don't need the else as you are going to return in the above.
> > 
> > Not always, only in case of error
> 
> Ok, but seems that it can be simplified a little.
> 
> What happens if a device uses more than one wait-pin? In other words a
> device with two chip-selects that uses two wait-pins?

Please re-read http://www.mail-archive.com/linux-omap@xxxxxxxxxxxxxxx/msg67702.html
and your reply

Regards
Afzal

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel