Ð ÐÑÐ, 09/02/2011 Ð 21:35 +0100, RafaÅ MiÅecki ÐÐÑÐÑ:
> 2011/2/9 George Kashperko <george@xxxxxxxxxxx>:
> > From: George Kashperko <george@xxxxxxxxxxx>
> >
> > Introduce handlers for SB core control and state flags' management. SB-style
> > buses provide access to these flags at two high octets of TMSLOW and TMSHIGH
> > registers whereas AI-ones implement these flags in two low octets of IOCTRL
> > and IOSTAT registers.
> > Signed-off-by: George Kashperko <george@xxxxxxxxxxx>
> > ---
> > drivers/ssb/main.c | 36 ++++++++++++++++++++++++++++++++++++
> > include/linux/ssb/ssb.h | 14 +++++++++++++-
> > 2 files changed, 49 insertions(+), 1 deletion(-)
> > --- linux-next-20110203.orig/drivers/ssb/main.c 2011-02-07 16:58:50.000000000 +0200
> > +++ linux-next-20110203/drivers/ssb/main.c 2011-02-07 17:07:11.000000000 +0200
> > @@ -1350,6 +1350,40 @@ static u32 ssb_admatch_size_sb(struct ss
> > return size;
> > }
> >
> > +static void ssb_core_ctl_flags_sb(struct ssb_device *dev, u32 mask,
> > + u32 val, u32 *res)
> > +{
> > + u32 tmp;
> > +
> > + if (mask || val) {
> > + tmp = (ssb_read32(dev, SSB_TMSLOW) & ~mask) | val;
> > + ssb_write32(dev, SSB_TMSLOW, tmp);
> > + }
>
> Are you going to use that function for just reading SSB_TMSLOW? Why
> won't you use separated function then? Do you need separated function
> for sth so simple as "ssb_read32(dev, SSB_TMSLOW);" at all?
Will answer your question with another question ;)
What would you personally prefer (press "1" or "2" :p)?
1)
if (sb_bus)
ssb_write32(dev, (ssb_read32(dev, SSB_TMSLOW) & ~(mask << 16)) | (flags << 16));
else
ssb_write32(dev, (ssb_read32(dev, SSB_AI_IOCTL) & ~mask) | flags));
2)
ssb_core_ctl_flags(dev, mask, flags, NULL);
>
> > + /* readback */
> > + tmp = ssb_read32(dev, SSB_TMSLOW);
> > + udelay(1);
> > +
> > + if (res)
> > + *res = tmp;
> > +}
>
> Why are you returning value by writing to pointed place? Can't you
> simply return it? Does returning complicate code so much, it is worth
> using that non-common solution?
>
If retval would be some memory location or this register accessor will
be used 99% of time just for reading (like ssb_core_state_flags is) I
would just <return retval;> like usual. Declared it in such (don't
really think its complicated, maybe just unusual?) manner just to speed
up things a little bit as io access is somewhat different from direct
(and mostly cached) memory access. This way of doing things changes
almost nothing for SB buses as TMSLOW requires readback to flush changes
posted, whereas readback for SSB_AI_IOCTL is not required and thus
return without reading will save some cpu time.
Not really huge optimisation, I know. Espesially considering how much
often it would be called. If you don't like it feel free to change void
to u32. Personally I dont like that u32 *res anyway ;)
Have nice day,
George
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
