On 5 May 2010 16:26, Sebastian Kuzminsky <[email protected]> wrote:

>> before. I guess that you are saying that memory is malloc-ed but not
>> freed in the case of an error?
>>
>
> That's it exactly.

Right, I will check through it all and look for such possibilities.

> The problem is that you first kmalloc memory for enable_reg, then pass
> enable_reg to hm2_register_tram_write_region().  That function allocates
> more memory and assigns it to the passed-in pointer-pointer.  So the
> kmalloced buffer is lost, and later when you kfree enable_reg in
> hm2_tp_pwmgen_cleanup() it'll segfault.

That sounds, inelegant.

> So initialization of tram regions can't happen in the parse_md function
> along with all the other initialization, it needs to be delayed until
> hm2_*_tram_init().

I will follow that through too.
>
> Also beware that TRAM memory gets read by hm2_read() each time it runs,
> so it'll overwrite anything your driver writes to that register-buffer
> at that time.  I think that's probably ok in your case.

I am not so sure. That buffer is bidirectional, holding both the
"enable-in" and "fault-out" bits.
Pete was discussing consolidating all "enables" for all instances in
one register and (presumably) all "fault" bits in another. Perhaps
that is a more important idea than I thought.

>> I took the view that there is always exactly one physical instance of
>> the LEDs, but perhaps there should be the option to not load the
>> module to save overhead.

> I'm not so much worried about the overhead of this, I think that's down
> in the noise.  This was just a reflexive reaction on my part, and feel
> free to ignore it: "there are exactly 8 LEDs per board" seems like a
> coincidence, and that limitation doesn't belong in the driver.

You would prefer the option to specify no instances of the LED driver,
and also that the driver supported future boards with an arbitrary
number of LEDs? The latter can presumably be handled in much the same
way as the num_dios parameter is handled in motmod and the number of
individual LEDs to create could be based on the board ID.
Not knowing how Mesa might choose to label the LEDs on these putative
future designs might make generating the names a bit of a lottery.
Currently it is a 32-bit buffer so adding many more LEDs is not
implausible.

-- 
atp

------------------------------------------------------------------------------
_______________________________________________
Emc-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/emc-developers

Reply via email to