Search Linux Wireless

Re: SSB AI support code ([RFC4/11] SSB core control and state device ops)

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

 



Ð ÐÑÐ, 09/02/2011 Ð 23:02 +0100, Michael BÃsch ÐÐÑÐÑ:
> On Wed, 2011-02-09 at 22:55 +0100, RafaÅ MiÅecki wrote: 
> > I aksed about reading, you gave me examples of writing. I want to
> > avoid such a non-readable disasters:
> > u32 tmp;
> > ssb_core_ctl_flags(dev, 0, 0, &tmp);
> 
> A good function name consists of:
> "WHAT is done and WHERE is it done"
> 
> ssb_core_ctl_flags()
> violates both of them. It doesn't specify what is done at all,
> except that it might be something random with ctl flags.
> And it lies about where it does it. It implies that it operates
> on SSB. However, it might operate on SSB or AI.
> 

Well, have nothing to say here.
Tbh things like I see them looks like following.
1. possibility is to "mask" bus differences by dev ops. Obviously you
already have seen these patches. This will cover pretty much everyting
on behalf of end device drivers. Everything _if_ decide on control/state
flags treatment.

2. Make separate bus driver. Not a bad way either I'm sure. But still
device drivers are to manage those control/state flags but this time
with if/elses (or just with sumthing like b43_ctl_flags/b43_state_flags
which will do the same as ssb_core_control_flags atm). Tbh b43_ctl_flags
would be bad name for such a function as donno if it gonna flags >> 16
or not so better do things with plain ssb_read32/ssb_write32 :p
As it would be separate bus there will be few more structs for
ssb_ai_driver_register or whatever, not worse mentinning but still. 

Apart from those flags' location everything else (dma, phy io, etc.)
will be pretty much the same. So the only difference from driver's
perespective here are
1. care control flags
2. care control flags, care bits more bus ops.

>From ssb code prespective for SB bus changes are little to nothing (does
it really matters that alot replacing extern fn1 with static inline
fn1(){ ops->fn_a1 }?) except that there will be 2 more files (ssb_ai.h,
ssb_ai.c 12k length both most of which is EROM scanning code - just take
a look at ssb_ai.c - 180 lines with copyrights and whitespace if you
delete that EROM dancing). Does that 180 lines really worse introducing
whole new bus ? Im not sure honestly but its like I see the things. And
ofcourse I dont insist on this vision.

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


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux