Re: [PATCH] iwl3945: added channel sysfs entry

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 2130 bytes --]

On Thu, 2008-09-25 at 14:04 +0300, Tomas Winkler wrote:

> > Therefore, I don't understand why you need to clutter the driver with
> > this code.
> 
> You are too development oriented not everybody is looking into kernel
> logs. It's easier to ask user
> to cat sysfs/debubgfs. This is not tracing this is just check static
> information.

Shouldn't be much harder to ask users to provide kernel logs, we've been
doing that with b43 forever.

> Malicious gossip has it you're trying to obfuscate the code
> > to a point where everybody else refuses to work on it.
> 
> We have probably most documented HW registers from all drivers so I
> doubt that has any ground :)

Well, in the code, I'm sure b43 has more documentation, I wrote it ;)

> We are definitely hooked to TX power, maybe you didn't notice

Didn't see it on iwl3945, but I only looked for the function that the
sysfs code calls and didn't find any other callsite, might well be a
different way.

> power_level
> >   also belongs here, sometimes one can't avoid the impression that
> >   you're adding things like that to avoid having to help define and
> >   implement the generic functionality;
> 
> The patches are no the way, actually  I've sent one yesterday and more
> are on the way.

yay :)

> except log level
> which I want always visible and has proven to be most useful.

Yeah, indeed, though it seems to be notably lacking printing out the
physical addresses and the ringbuffer entries. But that's just the thing
I was looking for yesterday when debugging the corruption issue.

> > generic option to disable hardware crypto in _mac80211_ rather than each
> > driver because this is actually genuinely useful if you want to monitor
> > frames at the same time.

> This is good point also disabling and enabling hw scan I would like to
> remove from module params.

That would be doable via mac80211 too, though that is a much more debug
option than disabling hardware crypto, I see disabling hw crypto as much
more useful than disabling hw scan.

> This is on TODO list.

:)

Thanks,
johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]