Skip to contentSkip to conversation

platform data and mfd design question

DISCUSSION Abhijeet Dharmapurikar 3 participants 4 messages Feb 15, 2011 – Feb 16, 2011 Arm Qualcomm mailing list

lore
Share & tools
lore
#0 lore raw

Currently all the mfd devices declare their struct mfd_cell sub_devices[] array within the core driver. The platform data to them is either passed in as a part of the core driver's platform data.

Msm on the other hand declares the struct mfd_cell subdevice[] array in the board file and passes this on to the core driver via platfom data. This code can be found here (sorry for the long url - it is convinient to click on it),
https://www.codeaurora.org/gitweb/quic/la/?p=kernel/msm.git;a=blob;f=arch/arm/mach-msm/board-msm8x60.c;h=ed9e9a7674b5ee443f25af828a0044ff99fac483;hb=refs/heads/android-msm-2.6.35
look for static struct mfd_cell pm8058_subdevs[]

This gives one the convenience of changing the mfd_cells and their platform data in the board file itself. There are boards where the platform data of some cells changes and in some cases we dont even add a particular cell.

This design makes the core driver very light weight. All it does is calls mfd_add_devices on the cell array passed from its platform data.

Will this be acceptable in mainline OR do we need to change to follow how others in drivers/mfd do it which is to define the mfd_cell array in the core file itself and manipulate their platform data before doing mfd_add_devices.

--
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

#1 lore raw
Quoted 2 lines · Abhijeet Dharmapurikar

This code can be found here (sorry for the long url - it is convinient to click on it),

I can't actually render this URL on a single line without fiddling with the font size for my terminal... When pasting gitweb URLs you almost always want to edit out the head ID and just rely on the branch.

Quoted 4 lines

This gives one the convenience of changing the mfd_cells and their platform data in the board file itself. There are boards where the platform data of some cells changes and in some cases we dont even add a particular cell.

My main question is why you're trying to do this. Even if some features aren't used on a given board one would strongly expect that the silicon will be the same anyway - what's the goal in changing the devices?

Quoted 3 lines

This design makes the core driver very light weight. All it does is calls mfd_add_devices on the cell array passed from its platform data.

The downside is that it then becomes impossible for the MFD to pass in resources to the devices (IRQs being the most obvious example) and every single board needs to replicate the definitions of all the subdevices which gets tedious, especially if you want to change them for some reason (eg, an IP becomes used in a wider range of chips so a rename is appropriate).

#2 lore raw
In reply to Abhijeet Dharmapurikar
Quoted 3 lines · Abhijeet Dharmapurikar

Msm on the other hand declares the struct mfd_cell subdevice[] array in the board file and passes this on to the core driver via platfom data.

This way the platform data tells the core driver what kind of silicon it has "hey, PM8058, guess what, you have an RTC!" which looks backwards to me, especially given that it does not need any fancy platform data at all, just two IRQ numbers which the core driver can very well handle.

For example: if the platform data (which is about how the components are connected on the board etc) does not provide the RTC resource, all of a sudden it appears to the system as if the PM8058 does not have an RTC, but it does...

Yours,
Linus Walleij

#3 lore raw
Quoted 9 lines · Linus Walleij
Quoted 2 lines · Abhijeet Dharmapurikar

Msm on the other hand declares the struct mfd_cell subdevice[] array in the board file and passes this on to the core driver via platfom data.

This way the platform data tells the core driver what kind of silicon it has "hey, PM8058, guess what, you have an RTC!" which looks backwards to me, especially given that it does not need any fancy platform data at all, just two IRQ numbers which the core driver can very well handle.

Indeed, and the RTC would still be useful even without IRQ support I imagine (it should still be able to tell you the time).