Re: [PATCH 28/61] Add cpm2_set_pin().

[Scott Wood <scottwood@freescale.com>]

Kumar Gala wrote:
> On Jul 17, 2007, at 8:35 PM, Scott Wood wrote:
>> +
>> +struct cpm2_ioports {
>> +    u32    dir, par, sor, odr, dat;
> 
> __be32?

OK.

>> +    u32    res[3];
>> +};
>> +
>> +void cpm2_set_pin(int port, int pin, int flags)
> 
> 
> Can we make the function take a pointer to the port directly?

I'd rather not -- it'd require the caller to use the immr struct 
directly, and either have no type checking, or different functions for 
different types of ports (on 8xx).

>> +{
>> +    struct cpm2_ioports __iomem *iop =
>> +        (struct cpm_ioports __iomem *)&cpm2_immr->im_ioport;
>> +
>> +    pin = 1 << (31 - pin);
>> +
>> +    if (flags & CPM_PIN_OUTPUT)
>> +        setbits32(&iop[port].dir, pin);
>> +    else
>> +        clrbits32(&iop[port].dir, pin);
>> +
>> +    if (!(flags & CPM_PIN_GPIO))
>> +        setbits32(&iop[port].par, pin);
>> +    else
>> +        clrbits32(&iop[port].par, pin);
>> +
>> +    if (flags & CPM_PIN_SECONDARY)
>> +        setbits32(&iop[port].sor, pin);
>> +    else
>> +        clrbits32(&iop[port].sor, pin);
> 
> 
> should we only do this if !(flags & CPM_PIN_GPIO)?

I don't think it matters.

> Any reason we don't also set odr here?

It wasn't clear to me when we need to set it.  I can add a flag for it, 
though.

-Scott