Re: [PATCH 2/3] MFD: AB8500 debugfs

[Mattias Wallin <mattias.wallin@stericsson.com>]

On 09/13/2010 03:26 PM, Samuel Ortiz wrote:
> Hi Mattias,
> 
> I have some addtional comments:
> 
> On Mon, Sep 13, 2010 at 10:33:02AM +0200, Mattias Wallin wrote:
>> +static struct platform_driver ab8500_debug_driver = {
>> +       .driver = {
>> +               .name = "ab8500-debug",
>> +               .owner = THIS_MODULE,
>> +       },
>> +       .probe  = ab8500_debug_probe,
>> +       .remove = __devexit_p(ab8500_debug_remove)
>> +};
>> +
>> +static int __init ab8500_debug_init(void)
>> +{
>> +       return platform_driver_register(&ab8500_debug_driver);
>> +}
>> +
>> +static void __exit ab8500_debug_exit(void)
>> +{
>> +       platform_driver_unregister(&ab8500_debug_driver);
>> +}
> It seems a bit awkward to me to have this code as an actual platform driver.
> Why not defining ab8500_debug_[probe|remove] in ab8500.h as no-op when
> CONFIG_AB8500_DEBUG is not defined and as extern otherwise ?
In the register access interface abx500_get_register_interruptible() the first
argument is a struct *device. This struct *device needs to be a (platform) device "child" to
the ab8500-core (dev->parent is used).
The idea is that we want only devices started by the ab8500 core to
be able to access the ab8500 registers. So all platform devices started by
ab8500-core just pass in their own struct *device as first argument.

So the register access restricted to device children is one reason. I also
want the debugfs driver to use the same access method as all other subdrivers.

BR,
/Mattias Wallin