| Message ID | 1303474109-6212-2-git-send-email-subhasish@mistralsolutions.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Mark, - Is it ok to have u32 etc for __iomem cookie ? > + > +s32 pruss_disable(struct device *dev, u8 pruss_num) make it a int function SG -- Ok will do > + > + /* Reset PRU */ > + iowrite32(PRUCORE_CONTROL_RESETVAL, > + &h_pruss->control); > + spin_unlock(&pruss->lock); > + > + return 0; make it a void function? SG -- This should be int, in case of invalid pru num, we ret an error. > +} > +EXPORT_SYMBOL_GPL(pruss_disable); > + > +s32 pruss_enable(struct device *dev, u8 pruss_num) int? SG -- Ok will do > + > + if ((pruss_num != PRUCORE_0) && (pruss_num != PRUCORE_1)) > + return -EINVAL; > + > + h_pruss = &pruss_mmap->core[pruss_num]; > + > + /* Reset PRU */ > + spin_lock(&pruss->lock); > + iowrite32(PRUCORE_CONTROL_RESETVAL, &h_pruss->control); no need to lock the ram reset below? SG -- I don't think its required. We just reset the RAM during init and since each pru can only be attached to only one device, the access will be already serialized based upon the pru num. > + /* Reset any garbage in the ram */ > + if (pruss_num == PRUCORE_0) > + for (i = 0; i < PRUSS_PRU0_RAM_SZ; i++) > + iowrite32(0x0, &pruss_mmap->dram0[i]); > + else if (pruss_num == PRUCORE_1) > + for (i = 0; i < PRUSS_PRU1_RAM_SZ; i++) > + iowrite32(0x0, &pruss_mmap->dram1[i]); if you make a array for these +struct pruss_map { + u8 dram0[512]; + u8 res1[7680]; + u8 dram1[512]; + u8 res2[7680]; ..} you don't need the if..else.. SG - The dram/iram is not contiguous, there is a reserved space in between, how do I declare an array for it. > +/* Load the specified PRU with code */ > +s32 pruss_load(struct device *dev, u8 pruss_num, > + u32 *pruss_code, u32 code_size_in_words) > +{ > + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); > + struct pruss_map __iomem *pruss_mmap = pruss->ioaddr; > + u32 __iomem *pruss_iram; > + u32 i; > + > + if (pruss_num == PRUCORE_0) > + pruss_iram = (u32 __iomem *)&pruss_mmap->iram0; > + else if (pruss_num == PRUCORE_1) > + pruss_iram = (u32 __iomem *)&pruss_mmap->iram1; > + else same here SG - same here. > +s32 pruss_run(struct device *dev, u8 pruss_num) int? SG - Ok, Will do. > + while (cnt--) { > + temp_reg = ioread32(&h_pruss->control); > + if (((temp_reg & PRUCORE_CONTROL_RUNSTATE_MASK) >> > + PRUCORE_CONTROL_RUNSTATE_SHIFT) == > + PRUCORE_CONTROL_RUNSTATE_HALT) > + break; how long might this take? what about some delay, sleep, or reschedule? SG - This does not take more than 10 to 20 counts, > +s32 pruss_writeb(struct device *dev, u32 offset, u8 pdatatowrite) > +{ > + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); > + void __iomem *paddresstowrite; we usually don't use "p" variable names for pointers SG - Ok, will remove. > +s32 pruss_rmwb(struct device *dev, u32 offset, u8 mask, u8 val) void function? SG - Ok, will do. > + > +s32 pruss_readb(struct device *dev, u32 offset, u8 *pdatatoread) void? SG - Ok, will do. > +{ > + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); > + void __iomem *paddresstoread; > + > + paddresstoread = pruss->ioaddr + offset ; > + *pdatatoread = ioread8(paddresstoread); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(pruss_readb); > + > +s32 pruss_readb_multi(struct device *dev, u32 offset, > + u8 *pdatatoread, u16 bytestoread) viod? SG - Ok, will do. > +{ > + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); > + u8 __iomem *paddresstoread; > + u16 i; int? SG - Ok, will do. > + return 0; > +} > +EXPORT_SYMBOL_GPL(pruss_readb_multi); > + > +s32 pruss_writel(struct device *dev, u32 offset, > + u32 pdatatowrite) void? SG - Ok, will do. > + return 0; > +} > +EXPORT_SYMBOL_GPL(pruss_writel); > + > +s32 pruss_writel_multi(struct device *dev, u32 offset, > + u32 *pdatatowrite, u16 wordstowrite) void? SG - Ok, will do. > +{ > + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); > + u32 __iomem *paddresstowrite; > + u16 i; > + > + paddresstowrite = pruss->ioaddr + offset; > + > + for (i = 0; i < wordstowrite; i++) > + iowrite32(*pdatatowrite++, paddresstowrite++); memcopy_to_iomem? SG -- I did not understand, could you please elaborate. > + > +s32 pruss_rmwl(struct device *dev, u32 offset, u32 mask, u32 val) void? SG - Ok, will do. > +} > +EXPORT_SYMBOL_GPL(pruss_rmwl); > + > +s32 pruss_readl(struct device *dev, u32 offset, u32 *pdatatoread) void? or return the read value SG - Ok, will do.
On Wed, Apr 27, 2011 at 09:29:59AM +0200, Marc Kleine-Budde wrote: > On 04/27/2011 08:39 AM, Subhasish Ghosh wrote: > > - Is it ok to have u32 etc for __iomem cookie ? > > no - "void __iomem *" is "void __iomem *" Actually, it is _provided_ you don't directly dereference it. You can then do pointer arithmetic on it in the usual way - which is about the only valid thing to do with an __iomem pointer. The voidness just acts as an additional check against direct dereferences of this. The important thing though is that the code passes sparse checks.
On Friday 22 April 2011, Subhasish Ghosh wrote: > This patch adds the pruss MFD driver and associated include files. > For details regarding the PRUSS please refer the folowing link: > http://processors.wiki.ti.com/index.php/Programmable_Realtime_Unit_Subsystem > > The rational behind the MFD driver being the fact that multiple devices can > be implemented on the cores independently. This is determined by the nature > of the program which is loaded into the PRU's instruction memory. > A device may be de-initialized and another loaded or two different devices > can be run simultaneously on the two cores. > It's also possible, as in our case, to implement a single device on both > the PRU's resulting in improved load sharing. > > Signed-off-by: Subhasish Ghosh <subhasish@mistralsolutions.com> Hi Subhasish, This looks like great progress since the last time I looked at the pruss mfd driver, good work there! Thanks to your explanations and the documentation link, I now have a better understanding of what is actually going on here, but I'd still like to understandhow the decision is made regarding what programs are loaded into each PRU and how the MFD cells are set up. Is this a fixed setting for each board that strictly depends on how the external pins are connected, or is it possible to use the same hardware for different purposes based on the program? If I read your code correctly, you hardwire the usage of the two PRUs in the da850 board code, which makes it impossible to use them in different ways even if the hardware supports it. If this is indeed the case, using an MFD device might not be the best option and we should try to come up with a way to dynamically repurpose the PRU with some user interface. Arnd
My problem is, I am doing something like this:
s32 pruss_writel_multi(struct device *dev, u32 offset,
u32 *pdatatowrite, u16 wordstowrite)
{
struct pruss_priv *pruss = dev_get_drvdata(dev->parent);
u32 __iomem *paddresstowrite;
u16 i;
paddresstowrite = pruss->ioaddr + offset;
for (i = 0; i < wordstowrite; i++)
iowrite32(*pdatatowrite++, paddresstowrite++);
return 0;
}
So, if I make paddresstowrite as void, it will not work.
The above implementation does not generate any sparse errors though.
> On Wed, Apr 27, 2011 at 09:29:59AM +0200, Marc Kleine-Budde wrote:
>> On 04/27/2011 08:39 AM, Subhasish Ghosh wrote:
>> > - Is it ok to have u32 etc for __iomem cookie ?
>>
>> no - "void __iomem *" is "void __iomem *"
>
> Actually, it is _provided_ you don't directly dereference it. You can
> then do pointer arithmetic on it in the usual way - which is about the
> only valid thing to do with an __iomem pointer. The voidness just acts
> as an additional check against direct dereferences of this.
>
> The important thing though is that the code passes sparse checks.
PRU's resulting in improved load sharing. >> >> Signed-off-by: Subhasish Ghosh <subhasish@mistralsolutions.com> > > Hi Subhasish, > > This looks like great progress since the last time I looked at the > pruss mfd driver, good work there! > > Thanks to your explanations and the documentation link, I now have > a better understanding of what is actually going on here, but I'd > still like to understandhow the decision is made regarding what programs > are loaded into each PRU and how the MFD cells are set up. > > Is this a fixed setting for each board that strictly depends on how > the external pins are connected, or is it possible to use the same > hardware for different purposes based on the program? SG -- The PRU pins are exposed as is on the main board, we have separate add on daughter cards specific to the protocol implemented on it. > > If I read your code correctly, you hardwire the usage of the two > PRUs in the da850 board code, which makes it impossible to use > them in different ways even if the hardware supports it. If this is > indeed the case, using an MFD device might not be the best option > and we should try to come up with a way to dynamically repurpose > the PRU with some user interface. SG -- It depends upon how the firmware is implemented. If another firmware is downloaded on it, it will emulate another device. Also, if a firmware emulated on it supports switching between devices, that too is possible. Its just a microcontroller, we can do whatever we feel like with it. Both the PRUs have separate instruction/data ram, so both can be used to implement two different devices.
On Wednesday 27 April 2011, Subhasish Ghosh wrote: > > > > If I read your code correctly, you hardwire the usage of the two > > PRUs in the da850 board code, which makes it impossible to use > > them in different ways even if the hardware supports it. If this is > > indeed the case, using an MFD device might not be the best option > > and we should try to come up with a way to dynamically repurpose > > the PRU with some user interface. > > SG -- It depends upon how the firmware is implemented. If another > firmware is downloaded on it, it will emulate another device. > Also, if a firmware emulated on it supports switching between > devices, > that too is possible. Its just a microcontroller, we can do > whatever we feel like > with it. Both the PRUs have separate instruction/data ram, so > both can be used > to implement two different devices. I see. So the problem that I see with the current code is that you force the system to provide a set of devices from the MFD, which then get passed to the individual drivers (uart and can) that load the firmware they need. Please correct me if I am reading your code wrong. What I suggest you do instead is to have the request_firmware call in the low-level MFD driver, so the user can provide the firmware that he/she wants to use, and then the MFD driver will create the devices that match the firmware loaded into the device. You can easily do that by adding a small header to the firmware format and interpret that header by the MFD driver. When the name of the subdevice is part of that header, the MFD driver does not need to understand the difference, it can simply pass that on when creating its child devices. Arnd
Hi, >> SG -- It depends upon how the firmware is implemented. If another >> firmware is downloaded on it, it will emulate another device. >> Also, if a firmware emulated on it supports switching between >> devices, >> that too is possible. Its just a microcontroller, we can do >> whatever we feel like >> with it. Both the PRUs have separate instruction/data ram, so >> both can be used >> to implement two different devices. > > I see. So the problem that I see with the current code is that you > force the system to provide a set of devices from the MFD, which > then get passed to the individual drivers (uart and can) that load > the firmware they need. Please correct me if I am reading your code > wrong. > > What I suggest you do instead is to have the request_firmware > call in the low-level MFD driver, so the user can provide the > firmware that he/she wants to use, and then the MFD driver will > create the devices that match the firmware loaded into the device. > > You can easily do that by adding a small header to the firmware > format and interpret that header by the MFD driver. When the name > of the subdevice is part of that header, the MFD driver does not > need to understand the difference, it can simply pass that on > when creating its child devices. I don't understand why loading the firmware should be done at the MFD driver. The user already specifies the device he/she wants to start on the PRU via modprobe. A driver can be inserted, which can download a printer firmware on one PRU and a scanner firmware on the other. This way both cores can be used for separate purposes. I mean, say in a real MFD controller, that will also have two separate cores running on it, just that, the firmware on it would not be downloaded runtime but fused in some non volatile memory.
Hi, On 04/27/2011 03:18 PM, Subhasish Ghosh wrote: > My problem is, I am doing something like this: > > s32 pruss_writel_multi(struct device *dev, u32 offset, > u32 *pdatatowrite, u16 wordstowrite) > { > struct pruss_priv *pruss = dev_get_drvdata(dev->parent); > u32 __iomem *paddresstowrite; > u16 i; > > paddresstowrite = pruss->ioaddr + offset; > > for (i = 0; i < wordstowrite; i++) > iowrite32(*pdatatowrite++, paddresstowrite++); > > return 0; > } > > So, if I make paddresstowrite as void, it will not work. The above > implementation does not generate any sparse errors though. Yes, that why I can work with readb_multi even if I have void __iomen *. But, how do I solve this problem. In the above function, I must use u32 __iomem *
On Thursday 28 April 2011 09:17:21 Subhasish Ghosh wrote: > > > > You can easily do that by adding a small header to the firmware > > format and interpret that header by the MFD driver. When the name > > of the subdevice is part of that header, the MFD driver does not > > need to understand the difference, it can simply pass that on > > when creating its child devices. > > I don't understand why loading the firmware should be done at the MFD > driver. > The user already specifies the device he/she wants to start on the PRU via > modprobe. > A driver can be inserted, which can download a printer firmware on one PRU > and a > scanner firmware on the other. This way both cores can be used for separate > purposes. > I mean, say in a real MFD controller, that will also have two separate cores > running on it, > just that, the firmware on it would not be downloaded runtime but fused in > some non volatile memory. Then I must be misreading what your code currently does, because it does not match your explanations. What I see in the platform code is that you create MFD cells for specific devices that get automatically created by the MFD driver. This will cause udev to load the drivers for these devices, which then load the firmware they need. Also, I cannot see how the method you describe would make it possible to the same driver into both units, e.g. when you want to have two serial ports. The reason is that you currently hardcode the PRU number in the driver and that you cannot load a single driver twice. Finally, I'm trying to make sure that whatever solution you come up with will still work when we migrate the code to using a flattened device tree. In that case, you would ideally put the device firmware into the device tree as a property that matches whatever you have connected on the specific board (at least as an option, you can still fall back to request_firmware). You definitely want automatic module loading in that case. Note that using module loading with specific parameters in order to match the hardware is not a recommended procedure any more. The code really needs to work the same way when all drivers are built into the kernel. It should not be hard to use the firmware loading mechanism in the MFD driver to both load the firmware and configure the devices appropriately so we always use the right driver for the currently active devices. Arnd BTW, something is wrong with your email client line wrapping. I've fixed this up manually before when replying, but please find a way to get this right in the future.
On Thursday 28 April 2011 09:22:49 Subhasish Ghosh wrote: > On 04/27/2011 03:18 PM, Subhasish Ghosh wrote: > > My problem is, I am doing something like this: > > > > s32 pruss_writel_multi(struct device *dev, u32 offset, > > u32 *pdatatowrite, u16 wordstowrite) > > { > > struct pruss_priv *pruss = dev_get_drvdata(dev->parent); > > u32 __iomem *paddresstowrite; > > u16 i; > > > > paddresstowrite = pruss->ioaddr + offset; > > > > for (i = 0; i < wordstowrite; i++) > > iowrite32(*pdatatowrite++, paddresstowrite++); > > > > return 0; > > } > > > > So, if I make paddresstowrite as void, it will not work. The above > > implementation does not generate any sparse errors though. > > Yes, that why I can work with readb_multi even if I have void __iomen *. > > But, how do I solve this problem. In the above function, I must use u32 > __iomem * > I believe you were talking about different things. The code you cited above looks correct to me. For clarity, I would write the loop as for (i = 0; i < wordstowrite; i++) iowrite32(pdatatowrite[i], &paddresstowrite[i]); but your version is just as correct, and I would not complain about it. It is absolutely valid to pass either a 'void __iomem *' or a 'u32 __iomem *' into iowrite32(). What is not valid is to cast between a 'void __iomem *' and a plain 'u32' (no pointer). While that may work in most cases, there are a lot of reasons why that is considered bad style and you should never write code like that. I believe that is what Marc was referring to, but you don't do it in your code. The initial comments that Marc made were about the return value of the accessor functions that always return success. Just make those return void instead. Again, this is unrelated to the pointer types. Arnd
Hi Arnd, How about just doing something like: #> echo da8xx_pruss_uart >> firmware.bin i.e just append the device name (from the board file) into the firmware file. In the driver probe, we can parse from the bottom, when it reaches "da8xx_pruss", the rest of the upper data is the firmware and from the full name, we can determine if it's a CAN, UART or any other peripheral. So, based on the platform_data, which the MFD driver received, it can find out which device to initialize. Also, does the line wrapping look any better ? > On Thursday 28 April 2011 09:17:21 Subhasish Ghosh wrote: >> > >> > You can easily do that by adding a small header to the firmware >> > format and interpret that header by the MFD driver. When the name >> > of the subdevice is part of that header, the MFD driver does not >> > need to understand the difference, it can simply pass that on >> > when creating its child devices. >> >> I don't understand why loading the firmware should be done at the MFD >> driver. >> The user already specifies the device he/she wants to start on the PRU >> via >> modprobe. >> A driver can be inserted, which can download a printer firmware on one >> PRU >> and a >> scanner firmware on the other. This way both cores can be used for >> separate >> purposes. >> I mean, say in a real MFD controller, that will also have two separate >> cores >> running on it, >> just that, the firmware on it would not be downloaded runtime but fused >> in >> some non volatile memory. > > Then I must be misreading what your code currently does, because it does > not > match your explanations. What I see in the platform code is that you > create > MFD cells for specific devices that get automatically created by the MFD > driver. This will cause udev to load the drivers for these devices, which > then load the firmware they need. > > Also, I cannot see how the method you describe would make it possible to > the same driver into both units, e.g. when you want to have two serial > ports. The reason is that you currently hardcode the PRU number in the > driver and that you cannot load a single driver twice. > > Finally, I'm trying to make sure that whatever solution you come up with > will still work when we migrate the code to using a flattened device tree. > In that case, you would ideally put the device firmware into the device > tree as a property that matches whatever you have connected on the > specific > board (at least as an option, you can still fall back to > request_firmware). > You definitely want automatic module loading in that case. > > Note that using module loading with specific parameters in order to > match the hardware is not a recommended procedure any more. The code > really needs to work the same way when all drivers are built into the > kernel. It should not be hard to use the firmware loading mechanism > in the MFD driver to both load the firmware and configure the devices > appropriately so we always use the right driver for the currently > active devices. > > Arnd > > BTW, something is wrong with your email client line wrapping. I've fixed > this up manually before when replying, but please find a way to get this > right in the future.
On Wednesday 04 May 2011, Subhasish Ghosh wrote: > How about just doing something like: > > #> echo da8xx_pruss_uart >> firmware.bin > > i.e just append the device name (from the board file) into the firmware > file. That sounds fine to me. I would put a header in the beginning, but feel free to use whatever format you find useful there. > In the driver probe, we can parse from the bottom, when it reaches > "da8xx_pruss", the rest of the upper data is the firmware and from > the full name, we can determine if it's a CAN, UART or any other > peripheral. > > So, based on the platform_data, which the MFD driver received, > it can find out which device to initialize. In my opinion, the MFD driver should not know the possible values at all, or look at platform_data for the children, but take all that information from the firmware file. The only thing that the MFD driver needs to know is how many PRUs there are and how they are connected to the host bus (memory regions, irqs, clocks, ...). It can then load firmware files, either one for the entire MFD or one per PRU core (whatever works best for you) and create child devices with the information it finds in there about what code to load into each PRU and what drivers to load for them. When you prepend the "da8xx_pruss_" string to the name you find in the firmware file, the pruss driver can be sure that it does not accidentally load a different module, e.g. when a malicious user was able to exchange the firmware file. > Also, does the line wrapping look any better ? Looks good now, yes. Arnd
Hi Subhasish, On Wed, May 04, 2011 at 12:48:43, Subhasish Ghosh wrote: > Hi Arnd, > > How about just doing something like: > > #> echo da8xx_pruss_uart >> firmware.bin > > i.e just append the device name (from the board file) into the firmware > file. > > In the driver probe, we can parse from the bottom, when it reaches > "da8xx_pruss", the rest of the upper data is the firmware and from > the full name, we can determine if it's a CAN, UART or any other > peripheral. > > So, based on the platform_data, which the MFD driver received, > it can find out which device to initialize. If you are defining a firmware header, it may be worthwhile making sure you include information on features of the device being supported (I assume there will be differences in CAN features if the firmware uses both PRUs to support CAN versus UART on one PRU and CAN on another PRU). The MFD driver can parse this information and pass this on to the UART or CAN drivers. Thanks, Sekhar > > Also, does the line wrapping look any better ? > > > > On Thursday 28 April 2011 09:17:21 Subhasish Ghosh wrote: > >> > > >> > You can easily do that by adding a small header to the firmware > >> > format and interpret that header by the MFD driver. When the name > >> > of the subdevice is part of that header, the MFD driver does not > >> > need to understand the difference, it can simply pass that on > >> > when creating its child devices. > >> > >> I don't understand why loading the firmware should be done at the MFD > >> driver. > >> The user already specifies the device he/she wants to start on the PRU > >> via > >> modprobe. > >> A driver can be inserted, which can download a printer firmware on one > >> PRU > >> and a > >> scanner firmware on the other. This way both cores can be used for > >> separate > >> purposes. > >> I mean, say in a real MFD controller, that will also have two separate > >> cores > >> running on it, > >> just that, the firmware on it would not be downloaded runtime but fused > >> in > >> some non volatile memory. > > > > Then I must be misreading what your code currently does, because it does > > not > > match your explanations. What I see in the platform code is that you > > create > > MFD cells for specific devices that get automatically created by the MFD > > driver. This will cause udev to load the drivers for these devices, which > > then load the firmware they need. > > > > Also, I cannot see how the method you describe would make it possible to > > the same driver into both units, e.g. when you want to have two serial > > ports. The reason is that you currently hardcode the PRU number in the > > driver and that you cannot load a single driver twice. > > > > Finally, I'm trying to make sure that whatever solution you come up with > > will still work when we migrate the code to using a flattened device tree. > > In that case, you would ideally put the device firmware into the device > > tree as a property that matches whatever you have connected on the > > specific > > board (at least as an option, you can still fall back to > > request_firmware). > > You definitely want automatic module loading in that case. > > > > Note that using module loading with specific parameters in order to > > match the hardware is not a recommended procedure any more. The code > > really needs to work the same way when all drivers are built into the > > kernel. It should not be hard to use the firmware loading mechanism > > in the MFD driver to both load the firmware and configure the devices > > appropriately so we always use the right driver for the currently > > active devices. > > > > Arnd > > > > BTW, something is wrong with your email client line wrapping. I've fixed > > this up manually before when replying, but please find a way to get this > > right in the future. > >
I am a bit lost, If I understand correctly, there are two problems 1. In case of udev based driver loading, we will end up loading both the drivers and the associated firmware. This is not acceptable because both UART & CAN use PRU0 & PRU1. 2. How do we switch between devices at run time. This is required because MFD devices should be capable of doing that. Ok, if that understanding is somewhat correct, then, what we are trying to do is load the firmware from the MFD driver itself. So, in case of a switch between device, say from UART to CAN, all the user has to do is copy a new firmware. This firmware should contain some info regarding the device, like say, device name & prunum. So, in case a device requires both PRUs, then the firmware would download for both PRUs. In case a device requires only one PRU, then it can be downloaded only into the requested PRU. So, the other PRU remains open for any other firmware/device. After loading the firmware, the MFD device should create the platform devices based upon the firmware. So, if only one PRU is used for UART, then the other PRU can be used for CAN and the MFD should create two platform_devices. If the UART uses both the PRUs, then the MFD should download the firmware into both the PRUs but create only one platform_device. But, in this case how do we download two different firmware on two different PRUs. One way is to keep the firmware files as its now, but add another file say firmware-config.txt, consisting a list of parameters like, START MFD_ID0#0 MFD_ID0_PRU_NUM#PRU0 MFD_ID1#1 MFD_ID1_PRU_NUM#PRU1 END OR START MFD_ID0#0 MFD_ID0_PRU_NUM#PRU0:PRU1 END So, the MFD driver can do a request_firmware on this file first, parse the device info and decide to request the main firmware. This becomes something like the devices table that we have on PPC. Also, if PRU0 is loaded, we need some way of maintaining the usage_count, So that the PRU0 is not loaded again accidently. We can increase the usage_count from the MFD driver. But, to decrement the usage_count, we can add an API which will be called when the CAN or UART driver is rmmod. Now how do we handle switch between devices. We can either do a rmmod of the MFD driver, then again modprobe it. So, the request_firmware is run again and the appropriate device is loaded based upon the changed firmware-config.txt. But, this does not look good, say what if the MFD driver is in-build. Another way is that we can add a sysfs entry, which when written with anything can re-evaluate the firmware_config. What the hell, does anything make any sense ? > Hi Subhasish, > > On Wed, May 04, 2011 at 12:48:43, Subhasish Ghosh wrote: >> Hi Arnd, >> >> How about just doing something like: >> >> #> echo da8xx_pruss_uart >> firmware.bin >> >> i.e just append the device name (from the board file) into the firmware >> file. >> >> In the driver probe, we can parse from the bottom, when it reaches >> "da8xx_pruss", the rest of the upper data is the firmware and from >> the full name, we can determine if it's a CAN, UART or any other >> peripheral. >> >> So, based on the platform_data, which the MFD driver received, >> it can find out which device to initialize. > > If you are defining a firmware header, it may be > worthwhile making sure you include information > on features of the device being supported (I > assume there will be differences in CAN features if > the firmware uses both PRUs to support CAN versus > UART on one PRU and CAN on another PRU). > > The MFD driver can parse this information and > pass this on to the UART or CAN drivers. > > Thanks, > Sekhar > >> >> Also, does the line wrapping look any better ? >> >> >> > On Thursday 28 April 2011 09:17:21 Subhasish Ghosh wrote: >> >> > >> >> > You can easily do that by adding a small header to the firmware >> >> > format and interpret that header by the MFD driver. When the name >> >> > of the subdevice is part of that header, the MFD driver does not >> >> > need to understand the difference, it can simply pass that on >> >> > when creating its child devices. >> >> >> >> I don't understand why loading the firmware should be done at the MFD >> >> driver. >> >> The user already specifies the device he/she wants to start on the PRU >> >> via >> >> modprobe. >> >> A driver can be inserted, which can download a printer firmware on one >> >> PRU >> >> and a >> >> scanner firmware on the other. This way both cores can be used for >> >> separate >> >> purposes. >> >> I mean, say in a real MFD controller, that will also have two separate >> >> cores >> >> running on it, >> >> just that, the firmware on it would not be downloaded runtime but >> >> fused >> >> in >> >> some non volatile memory. >> > >> > Then I must be misreading what your code currently does, because it >> > does >> > not >> > match your explanations. What I see in the platform code is that you >> > create >> > MFD cells for specific devices that get automatically created by the >> > MFD >> > driver. This will cause udev to load the drivers for these devices, >> > which >> > then load the firmware they need. >> > >> > Also, I cannot see how the method you describe would make it possible >> > to >> > the same driver into both units, e.g. when you want to have two serial >> > ports. The reason is that you currently hardcode the PRU number in the >> > driver and that you cannot load a single driver twice. >> > >> > Finally, I'm trying to make sure that whatever solution you come up >> > with >> > will still work when we migrate the code to using a flattened device >> > tree. >> > In that case, you would ideally put the device firmware into the device >> > tree as a property that matches whatever you have connected on the >> > specific >> > board (at least as an option, you can still fall back to >> > request_firmware). >> > You definitely want automatic module loading in that case. >> > >> > Note that using module loading with specific parameters in order to >> > match the hardware is not a recommended procedure any more. The code >> > really needs to work the same way when all drivers are built into the >> > kernel. It should not be hard to use the firmware loading mechanism >> > in the MFD driver to both load the firmware and configure the devices >> > appropriately so we always use the right driver for the currently >> > active devices. >> > >> > Arnd >> > >> > BTW, something is wrong with your email client line wrapping. I've >> > fixed >> > this up manually before when replying, but please find a way to get >> > this >> > right in the future. >> >> >
On Thursday 05 May 2011, Subhasish Ghosh wrote: > I am a bit lost, > > If I understand correctly, there are two problems > > 1. In case of udev based driver loading, we will end up > loading both the drivers and the associated firmware. > This is not acceptable because both UART & CAN > use PRU0 & PRU1. > > 2. How do we switch between devices at run time. > This is required because MFD devices should be > capable of doing that. > > > Ok, if that understanding is somewhat correct, then, what we are > trying to do is load the firmware from the MFD driver itself. I would describe the problem differently: You hardcode the usage of the pruss in the platform code, even though it is completely defined by the software you load into it. I think it is essential that all the configuration of the pruss is dynamic and not hardcoded in the platform code, so that it is at all possible to load other firmware into it. Describing the device in the firmware file instead of the platform code is one way to get there, but there may be other ways that you could come up with. > So, in case of a switch between device, say from UART to CAN, > all the user has to do is copy a new firmware. > > This firmware should contain some info regarding the device, like > say, device name & prunum. Right. > So, in case a device requires both PRUs, then the firmware would > download for both PRUs. > > In case a device requires only one PRU, then it can be downloaded > only into the requested PRU. So, the other PRU remains open for > any other firmware/device. > > After loading the firmware, the MFD device should create the > platform devices based upon the firmware. > > So, if only one PRU is used for UART, then the other PRU can be > used for CAN and the MFD should create two platform_devices. > > If the UART uses both the PRUs, then the MFD should download > the firmware into both the PRUs but create only one platform_device. As I mentioned, I did not have a good idea for how to abstract the fact that there may be either one or two child devices, depending on the firmware. A possible way to deal with this would be to have only a single firmware image files that contains the code for both PRUs, with a header indicating how many devices there should be and what they are called. > But, in this case how do we download two different firmware > on two different PRUs. One way is to keep the firmware files > as its now, but add another file say firmware-config.txt, > consisting a list of parameters like, > > START > MFD_ID0#0 > MFD_ID0_PRU_NUM#PRU0 > MFD_ID1#1 > MFD_ID1_PRU_NUM#PRU1 > END > > OR > > START > MFD_ID0#0 > MFD_ID0_PRU_NUM#PRU0:PRU1 > END > > So, the MFD driver can do a request_firmware on this file first, > parse the device info and decide to request the main firmware. > This becomes something like the devices table that we have on > PPC. Yes, this would be another option. I generally don't like parsing data in the kernel, but it's not completely unrealistic. > Also, if PRU0 is loaded, we need some way of maintaining the > usage_count, So that the PRU0 is not loaded again accidently. > We can increase the usage_count from the MFD driver. > But, to decrement the usage_count, we can add an API > which will be called when the CAN or UART driver is rmmod. Not necesarily. You can make the interface so that the child device gets registered after the firmware got loaded, but unregistered if you start loading new firmware. We have hotplug support for a lot of devices, and it's not hard to write the serial/can/... drivers in a way that allows the underlying device to go away. You should not actually need to unload the device driver. E.g. when the drivers are built into the kernel, you can still remove the device and put it back in there using the ->probe and ->remove callbacks of the platform driver. > Now how do we handle switch between devices. > > We can either do a rmmod of the MFD driver, then again modprobe it. > So, the request_firmware is run again and the appropriate device is > loaded based upon the changed firmware-config.txt. > > But, this does not look good, say what if the MFD driver is in-build. > Another way is that we can add a sysfs entry, which when written > with anything can re-evaluate the firmware_config. Yes, I think that a sysfs entry would be good here, basically to reset the entire unit and remove the child devices, followed by a new call to request_firmware. Instead of passing a configuration file into the MFD driver and calling it a firmware, I can see three other options that I believe would be nicer: 1. have a single firmware blob that describes the child devices and the code that is to be loaded into both units. If they are acting as one device, either make sure you only create one child node, or create one with a name that no driver binds to. 2. Call request_firmware separately for the two child devices, and configure them separately based on the information in the firmware files. In the case where they should act as a single device, call the children e.g. "pruss-uart-a" and "pruss-uart-b", then bind to both names in the pruss-uart drivers and only start using the device when you have both nodes for the same parent. 3. Do the configuration through sysfs files in the MFD device node, which then cause the creation of the child devices. This means you need extra user space scripts to write the addititonal files, but is also the most flexible way. Arnd
> Instead of passing a configuration file into the MFD driver and > calling it a firmware, I can see three other options that I believe > would be nicer: > > 1. have a single firmware blob that describes the child devices > and the code that is to be loaded into both units. If they are > acting as one device, either make sure you only create one > child node, or create one with a name that no driver binds to. > > 2. Call request_firmware separately for the two child devices, > and configure them separately based on the information in the > firmware files. In the case where they should act as a single > device, call the children e.g. "pruss-uart-a" and "pruss-uart-b", > then bind to both names in the pruss-uart drivers and only > start using the device when you have both nodes for the same > parent. > > 3. Do the configuration through sysfs files in the MFD device node, > which then cause the creation of the child devices. This means you > need extra user space scripts to write the addititonal files, but > is also the most flexible way. > Are you suggesting something like: /sys..../pruss/pru0/id /sys..../pruss/pru0/fw_name /sys..../pruss/pru0/load /sys..../pruss/pru1/id /sys..../pruss/pru1/fw_name /sys..../pruss/pru1/load I can program these configs and store them in the pru private. Then write something into load. This can load the PRU based upon the configs. But, I am not sure how to do this effectively using sysfs, I mean I don't want to end up writing six write and six read handlers.
On Tuesday 10 May 2011, Subhasish Ghosh wrote: > > Instead of passing a configuration file into the MFD driver and > > calling it a firmware, I can see three other options that I believe > > would be nicer: > > > > 1. have a single firmware blob that describes the child devices > > and the code that is to be loaded into both units. If they are > > acting as one device, either make sure you only create one > > child node, or create one with a name that no driver binds to. > > > > 2. Call request_firmware separately for the two child devices, > > and configure them separately based on the information in the > > firmware files. In the case where they should act as a single > > device, call the children e.g. "pruss-uart-a" and "pruss-uart-b", > > then bind to both names in the pruss-uart drivers and only > > start using the device when you have both nodes for the same > > parent. > > > > 3. Do the configuration through sysfs files in the MFD device node, > > which then cause the creation of the child devices. This means you > > need extra user space scripts to write the addititonal files, but > > is also the most flexible way. > > > > Are you suggesting something like: > > /sys..../pruss/pru0/id > /sys..../pruss/pru0/fw_name > /sys..../pruss/pru0/load > > /sys..../pruss/pru1/id > /sys..../pruss/pru1/fw_name > /sys..../pruss/pru1/load No, that is none of the three I suggested. > I can program these configs and store them in the pru private. > Then write something into load. > This can load the PRU based upon the configs. > But, I am not sure how to do this effectively using sysfs, > I mean I don't want to end up writing six write and six read > handlers. Please look into implementing one of the three I suggested before you go off in another direction. In case of the third one, the idea was to configure the name of the device for each pru using sysfs, which then gets bound to the driver, which loads its own firmware as you do today. Only in the first two suggestions, the mfd driver would be responsible for loading the firmware. Arnd
> Please look into implementing one of the three I suggested before > you go off in another direction. In case of the third one, the idea > was to configure the name of the device for each pru using sysfs, > which then gets bound to the driver, which loads its own firmware > as you do today. Only in the first two suggestions, the mfd driver > would be responsible for loading the firmware. Ok, thanks for the clarification. Instead of passing the device name, will it be ok to pass the mfd_id. The benefit will be that I can use the ID directly as an array index for the mfd_cell entries. Just to verify my understanding, all that's needs to be done is add a sysfs entry at /sys/devices/platform/pruss_mfd/mfd_id Writing to this entry would create the respective device. Rewriting would unload the old (mfd_remove_device) and reload the new device. Reading may return the various devices and their name aliases.
On Wednesday 11 May 2011, Subhasish Ghosh wrote: > > Please look into implementing one of the three I suggested before > > you go off in another direction. In case of the third one, the idea > > was to configure the name of the device for each pru using sysfs, > > which then gets bound to the driver, which loads its own firmware > > as you do today. Only in the first two suggestions, the mfd driver > > would be responsible for loading the firmware. > > Ok, thanks for the clarification. > Instead of passing the device name, will it be ok to pass the mfd_id. > The benefit will be that I can use the ID directly as an array > index for the mfd_cell entries. I think a device name would be clearer here, especially in order to avoid conflicts when the list gets extended in different ways depending on which kernel runs. We had a little discussion at the Linaro Developer Summit about your driver and mfd drivers in general. There was a general feeling among some people (including me) that by the point you dynamically create the subdevices, MFD is probably not the right abstraction any more, as it does not provide any service that you need. Instead, maybe you can simply call platform_device_register at that stage to create the children and not use MFD at all. Samuel, can you comment on this as well? Do you still see pruss as an MFD driver when the uses are completely dynamic and determined by the firmware loaded into it? > Just to verify my understanding, all that's needs to be done is add > a sysfs entry at > /sys/devices/platform/pruss_mfd/mfd_id > > Writing to this entry would create the respective device. > Rewriting would unload the old (mfd_remove_device) > and reload the new device. Yes > Reading may return the various devices and their name > aliases. I would just return the current value. You might want to have two files in sysfs, one per PRU, so you can set them individually. Arnd
>> >> Ok, thanks for the clarification. >> Instead of passing the device name, will it be ok to pass the mfd_id. >> The benefit will be that I can use the ID directly as an array >> index for the mfd_cell entries. > > I think a device name would be clearer here, especially in order > to avoid conflicts when the list gets extended in different ways > depending on which kernel runs. > > We had a little discussion at the Linaro Developer Summit about your > driver and mfd drivers in general. There was a general feeling among > some people (including me) that by the point you dynamically create > the subdevices, MFD is probably not the right abstraction any more, > as it does not provide any service that you need. > > Instead, maybe you can simply call platform_device_register > at that stage to create the children and not use MFD at all. > > Samuel, can you comment on this as well? Do you still see pruss > as an MFD driver when the uses are completely dynamic and determined > by the firmware loaded into it? > Hi Arnd, But in that case, where do I fit my driver. It's a microcontroller inside of a processor, guess that's unique for Linux. Will a misc device be a better choice. In that case I can remove the MFD cell from the board_file and add an array of platform_device for the UART & CAN. Regarding device register, I will have to implement something similar to mfd_add_devices and mark the dev->parent. Did I miss something or will that be fine.
On Fri, May 13, 2011 at 04:25:56PM +0530, Subhasish Ghosh wrote: > >Instead, maybe you can simply call platform_device_register > >at that stage to create the children and not use MFD at all. > >Samuel, can you comment on this as well? Do you still see pruss > >as an MFD driver when the uses are completely dynamic and determined > >by the firmware loaded into it? > But in that case, where do I fit my driver. > It's a microcontroller inside of a processor, guess that's unique for Linux. It's not that unusual in hardware terms, really. There's an awful lot of this going on in the audio area especially at the minute so there's going to be some effort around providing some framework support for this which should have some general usability but that's not there yet. > Will a misc device be a better choice. > In that case I can remove the MFD cell from the board_file and add an array > of platform_device for the UART & CAN. I think that for me so long as it can simultaneously do two unrelated functions which need to be arbitrated between there's probably a place for it in MFD, certainly at the minute.
On Saturday 14 May 2011, Mark Brown wrote: > > Will a misc device be a better choice. > > > In that case I can remove the MFD cell from the board_file and add an array > > of platform_device for the UART & CAN. > > I think that for me so long as it can simultaneously do two unrelated > functions which need to be arbitrated between there's probably a place > for it in MFD, certainly at the minute. I guess drivers/mfd would be a better place than drivers/misc, but it might not need to be an mfd driver in the sense that it registers mfd cells. The more important point is to remove the device registration from the board file. You definitely should not have the cells in the board file, but replacing them with platform devices in the board file makes it no better. My whole point has been that you register them from the main pruss driver based on run-time data instead of compile-time pre-configured stuff in the board file. Arnd
On Sat, May 14, 2011 at 10:33:53PM +0200, Arnd Bergmann wrote: > I guess drivers/mfd would be a better place than drivers/misc, but it might not > need to be an mfd driver in the sense that it registers mfd cells. Yes, it might be more sensible to just open code. OTOH mfd_cell is marginally easier to use. > The more important point is to remove the device registration from the board > file. You definitely should not have the cells in the board file, but replacing > them with platform devices in the board file makes it no better. Agreed entirely, it should be something higher level than that. > My whole point has been that you register them from the main pruss driver > based on run-time data instead of compile-time pre-configured stuff in the > board file. I'm not so sure - if the usage is fixed as a result of the pins on the device being wired a CAN bus then it seems reasonable to tell the system about that so it'll stop the user trying to run SPI or something against it at runtime.
On Sunday 15 May 2011, Mark Brown wrote: > > My whole point has been that you register them from the main pruss driver > > based on run-time data instead of compile-time pre-configured stuff in the > > board file. > > I'm not so sure - if the usage is fixed as a result of the pins on the > device being wired a CAN bus then it seems reasonable to tell the system > about that so it'll stop the user trying to run SPI or something against > it at runtime. I'm mostly worried about the case where the pins are not hardwired for some specific function -- Subhasish was mentioning that these may be implemented using a pluggable extension board and I want to make sure that you are not required to recompile the kernel when changing the extension board. However, you made a good point that in many cases it will be hardwired so it may be valuable to preconfigure this in a way that does not require scripts to set up variables in sysfs when you already know what is there. Note that my suggestion to put the device name into the firmware file covers this case, because you can then simply ship a firmware blob that matches the hardware configuration. Thinking about the future device tree setup, you can even put the firmware blob itself into a property in the device tree file. Arnd
Hi!, >> > My whole point has been that you register them from the main pruss >> > driver >> > based on run-time data instead of compile-time pre-configured stuff in >> > the >> > board file. >> >> I'm not so sure - if the usage is fixed as a result of the pins on the >> device being wired a CAN bus then it seems reasonable to tell the system >> about that so it'll stop the user trying to run SPI or something against >> it at runtime. > > I'm mostly worried about the case where the pins are not hardwired for > some specific function -- Subhasish was mentioning that these may be > implemented using a pluggable extension board and I want to make sure > that you are not required to recompile the kernel when changing the > extension board. > > However, you made a good point that in many cases it will be hardwired > so it may be valuable to preconfigure this in a way that does not require > scripts to set up variables in sysfs when you already know what is there. > > Note that my suggestion to put the device name into the firmware file > covers this case, because you can then simply ship a firmware blob that > matches the hardware configuration. Thinking about the future device > tree setup, you can even put the firmware blob itself into a property > in the device tree file. > I earlier had an implementation where I used a pruss_devices structure in the board file. http://linux.omap.com/pipermail/davinci-linux-open-source/ 2011-March/022339.html. We can use this implementation along with the sysfs to load the devices runtime. The configs that I have in the board_file for the devices structure, are fixed for a board. To swap the boards, we do not need to re-compile the kernel.
Hi Arnd, On Wed, May 11, 2011 at 10:03:54PM +0200, Arnd Bergmann wrote: > On Wednesday 11 May 2011, Subhasish Ghosh wrote: > > > Please look into implementing one of the three I suggested before > > > you go off in another direction. In case of the third one, the idea > > > was to configure the name of the device for each pru using sysfs, > > > which then gets bound to the driver, which loads its own firmware > > > as you do today. Only in the first two suggestions, the mfd driver > > > would be responsible for loading the firmware. > > > > Ok, thanks for the clarification. > > Instead of passing the device name, will it be ok to pass the mfd_id. > > The benefit will be that I can use the ID directly as an array > > index for the mfd_cell entries. > > I think a device name would be clearer here, especially in order > to avoid conflicts when the list gets extended in different ways > depending on which kernel runs. > > We had a little discussion at the Linaro Developer Summit about your > driver and mfd drivers in general. There was a general feeling among > some people (including me) that by the point you dynamically create > the subdevices, MFD is probably not the right abstraction any more, > as it does not provide any service that you need. I agree it's not what it's been designed for. > Instead, maybe you can simply call platform_device_register > at that stage to create the children and not use MFD at all. The MFD APIs are slightly easier to use though, imho. > Samuel, can you comment on this as well? Do you still see pruss > as an MFD driver when the uses are completely dynamic and determined > by the firmware loaded into it? Even though that is definitely not a typical MFD use case, I wouldn't object strongly against it. Right now mfd is probably the least worst choice for this kind of drivers, which still doesn't make it an ideal situation. Cheers, Samuel.
Hi Arnd, On Sat, May 14, 2011 at 10:33:53PM +0200, Arnd Bergmann wrote: > On Saturday 14 May 2011, Mark Brown wrote: > > > Will a misc device be a better choice. > > > > > In that case I can remove the MFD cell from the board_file and add an array > > > of platform_device for the UART & CAN. > > > > I think that for me so long as it can simultaneously do two unrelated > > functions which need to be arbitrated between there's probably a place > > for it in MFD, certainly at the minute. > > I guess drivers/mfd would be a better place than drivers/misc, but it might not > need to be an mfd driver in the sense that it registers mfd cells. > I agree with that. Although if it makes it to drivers/mfd/, I'd prefer to see it using the MF API. > The more important point is to remove the device registration from the board > file. You definitely should not have the cells in the board file, but replacing > them with platform devices in the board file makes it no better. > > My whole point has been that you register them from the main pruss driver > based on run-time data instead of compile-time pre-configured stuff in the > board file. That's certainly right. Cheers, Samuel.
Hi Samuel, On Sunday 22 May 2011, Samuel Ortiz wrote: > On Wed, May 11, 2011 at 10:03:54PM +0200, Arnd Bergmann wrote: > > On Wednesday 11 May 2011, Subhasish Ghosh wrote: > > We had a little discussion at the Linaro Developer Summit about your > > driver and mfd drivers in general. There was a general feeling among > > some people (including me) that by the point you dynamically create > > the subdevices, MFD is probably not the right abstraction any more, > > as it does not provide any service that you need. > I agree it's not what it's been designed for. > > > > Instead, maybe you can simply call platform_device_register > > at that stage to create the children and not use MFD at all. > The MFD APIs are slightly easier to use though, imho. > > > > Samuel, can you comment on this as well? Do you still see pruss > > as an MFD driver when the uses are completely dynamic and determined > > by the firmware loaded into it? > Even though that is definitely not a typical MFD use case, I wouldn't object > strongly against it. Right now mfd is probably the least worst choice for this > kind of drivers, which still doesn't make it an ideal situation. Thanks for your input! When you say that MFD APIs are easier to use than platform_device APIs, is that something we should fix with platform devices to make them easier to use? Arnd
On Monday 16 May 2011, Subhasish Ghosh wrote: > I earlier had an implementation where I used a pruss_devices structure > in the board file. > > http://linux.omap.com/pipermail/davinci-linux-open-source/ > 2011-March/022339.html. > > We can use this implementation along with the sysfs to load the devices > runtime. Possibly, but the actual data structures might end up differently when they are built around a sysfs interface. If you have a sysfs interface, it is more important to have that in a clean way than the board file, so we should first discuss the set of sysfs attributes that you are going to need, and then see how to represent that in platform data for predefined boards. > The configs that I have in the board_file for the devices > structure, are fixed for a board. To swap the boards, we do not need to re-compile > the kernel. The other point to consider is that we are definitely moving towards the flattened device tree for these definitions now. It's probably good to make the sysfs attributes directly correspond to fdt device properties. I'm not sure if we also need to allow platform data. The easiest way could be to just require the use of device tree for predefined pruss devices. I'm sorry that this is moving in a different direction now, you had an unfortunate timing here. Let's first discuss the exact properties that are really required to define the differences between PRU backends, as those will be required in any case. What do you need for PRU specific data besides the firmware and the name of the device? Arnd
Hi Arnd, >> http://linux.omap.com/pipermail/davinci-linux-open-source/ >> 2011-March/022339.html. >> >> We can use this implementation along with the sysfs to load the devices >> runtime. > > Possibly, but the actual data structures might end up differently > when they are built around a sysfs interface. If you have a sysfs > interface, it is more important to have that in a clean way than > the board file, so we should first discuss the set of sysfs > attributes that you are going to need, and then see how to > represent that in platform data for predefined boards. > >> The configs that I have in the board_file for the devices >> structure, are fixed for a board. To swap the boards, we do not need to >> re-compile >> the kernel. > > The other point to consider is that we are definitely moving > towards the flattened device tree for these definitions now. > It's probably good to make the sysfs attributes directly correspond > to fdt device properties. I'm not sure if we also need to allow platform > data. The easiest way could be to just require the use of device tree > for predefined pruss devices. > > I'm sorry that this is moving in a different direction now, you > had an unfortunate timing here. > > Let's first discuss the exact properties that are really required > to define the differences between PRU backends, as those will > be required in any case. What do you need for PRU specific > data besides the firmware and the name of the device? Would it be a good approach to first get a basic sensible driver into the tree and then work towards improving and adjusting for future compatibilities. That way we can gradually build towards the perfect driver in steps, rather than digressing far too off suddenly. That would be some source of motivation for me too.
On Tuesday 24 May 2011, Subhasish Ghosh wrote: > Would it be a good approach to first get a basic sensible > driver into the tree and then work towards improving and > adjusting for future compatibilities. The main problem with that is that you cannot really change user-visible interfaces after the driver is merged. Anything regarding sysfs attributes and firmware file contents needs to be fixed. You can however still change in-kernel interfaces at a later point without problems. > That way we can gradually build towards the perfect driver > in steps, rather than digressing far too off suddenly. > That would be some source of motivation for me too. You can definitely add the driver to drivers/staging/pruss/, as the rules for staging drivers are different. I think that would indeed be helpful at this point. As soon as all interfaces have stabilized, you can then move the individual parts to their final location. Greg maintains the staging drivers, and he can surely point you to a document describing what is required for a driver. Mainly, this includes having a patch that adds a single directory with all the driver files under drivers/staging. The driver must be able to compile without errors and you need a TODO file listing the remaining issues that prevent you from having a non-staging driver. I think the list will be relatively short, given that you have made a lot of progress and the driver is now essentially ready, aside from a few details and the big question of how the firmware should be configured and loaded. Arnd
On Tue, May 24, 2011 at 02:40:34PM +0200, Arnd Bergmann wrote: > On Tuesday 24 May 2011, Subhasish Ghosh wrote: > > Would it be a good approach to first get a basic sensible > > driver into the tree and then work towards improving and > > adjusting for future compatibilities. > > The main problem with that is that you cannot really change > user-visible interfaces after the driver is merged. Anything > regarding sysfs attributes and firmware file contents needs > to be fixed. You can however still change in-kernel interfaces > at a later point without problems. > > > That way we can gradually build towards the perfect driver > > in steps, rather than digressing far too off suddenly. > > That would be some source of motivation for me too. > > You can definitely add the driver to drivers/staging/pruss/, > as the rules for staging drivers are different. I think that > would indeed be helpful at this point. As soon as all > interfaces have stabilized, you can then move the individual > parts to their final location. > > Greg maintains the staging drivers, and he can surely point > you to a document describing what is required for a driver. There really are only 3 rules: - proper license - self-contained in a drivers/staging/DRIVER_NAME/ directory - must properly build > Mainly, this includes having a patch that adds a single > directory with all the driver files under drivers/staging. > The driver must be able to compile without errors and you need > a TODO file listing the remaining issues that prevent you > from having a non-staging driver. Ah, forgot the TODO on the list of rules, I'll have to add that next time. Someone feel free to send me a patch :) thanks, greg k-h
Greg, > There really are only 3 rules: > - proper license > - self-contained in a drivers/staging/DRIVER_NAME/ directory > - must properly build > >> Mainly, this includes having a patch that adds a single >> directory with all the driver files under drivers/staging. >> The driver must be able to compile without errors and you need >> a TODO file listing the remaining issues that prevent you >> from having a non-staging driver. > > Ah, forgot the TODO on the list of rules, I'll have to add that next > time. > How do I handle the headers. I have two header files in the include/linux/mfd. Should I submit them as a separate patch into mfd. These headers are also used by pruss uart implementation.
On Mon, May 30, 2011 at 06:55:12PM +0530, Subhasish Ghosh wrote: > Greg, > > >There really are only 3 rules: > >- proper license > >- self-contained in a drivers/staging/DRIVER_NAME/ directory > >- must properly build > > > >>Mainly, this includes having a patch that adds a single > >>directory with all the driver files under drivers/staging. > >>The driver must be able to compile without errors and you need > >>a TODO file listing the remaining issues that prevent you > >>from having a non-staging driver. > > > >Ah, forgot the TODO on the list of rules, I'll have to add that next > >time. > > > How do I handle the headers. I have two header files in the > include/linux/mfd. Why would they be in there for a single driver? > Should I submit them as a separate patch into mfd. > > These headers are also used by pruss uart implementation. Then put them in the staging directory for your driver, why would anything outside of your driver need a .h file? thanks, greg k-h
On Monday 30 May 2011, Subhasish Ghosh wrote: > > Ah, forgot the TODO on the list of rules, I'll have to add that next > > time. > > > How do I handle the headers. I have two header files in the > include/linux/mfd. > > Should I submit them as a separate patch into mfd. > > These headers are also used by pruss uart implementation. During the time when the drivers are in staging, all files need to be in the same directory, including the header and the drivers using it. They can get moved at the time when the driver gets turned into a regular non-staging driver. Arnd
> On Monday 30 May 2011, Subhasish Ghosh wrote: >> > Ah, forgot the TODO on the list of rules, I'll have to add that next >> > time. >> > >> How do I handle the headers. I have two header files in the >> include/linux/mfd. >> >> Should I submit them as a separate patch into mfd. >> >> These headers are also used by pruss uart implementation. > > During the time when the drivers are in staging, all files need to > be in the same directory, including the header and the drivers using it. > They can get moved at the time when the driver gets turned into > a regular non-staging driver. OK, so the UART implementation needs to be moved into staging as well ? And what about the boardfile changes, array of mfd_cells implementation etc.
Greg, >> > >> How do I handle the headers. I have two header files in the >> include/linux/mfd. > > Why would they be in there for a single driver? > >> Should I submit them as a separate patch into mfd. >> >> These headers are also used by pruss uart implementation. > > Then put them in the staging directory for your driver, why would > anything outside of your driver need a .h file? > There are two header files in the include/linux/mfd/pruss.h & pruss_core.h the file pruss.h is included by the pruss_suart_api.c (tty uart driver) for some data structures, defines and function prototypes.
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 0284c53..41479e4 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -92,6 +92,16 @@ config MFD_TI_SSP To compile this driver as a module, choose M here: the module will be called ti-ssp. +config MFD_DA8XX_PRUSS + tristate "Texas Instruments DA8XX PRUSS support" + depends on ARCH_DAVINCI_DA850 + select MFD_CORE + help + This driver provides support API for the programmable + realtime unit (PRU) present on TI's da8xx processors. It + provides basic read, write, config, enable, disable + routines to facilitate devices emulated on it. + config HTC_EGPIO bool "HTC EGPIO support" depends on GENERIC_HARDIRQS && GPIOLIB && ARM diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index c56b6c7..8015dea 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -13,6 +13,7 @@ obj-$(CONFIG_HTC_PASIC3) += htc-pasic3.o obj-$(CONFIG_HTC_I2CPLD) += htc-i2cpld.o obj-$(CONFIG_MFD_DAVINCI_VOICECODEC) += davinci_voicecodec.o +obj-$(CONFIG_MFD_DA8XX_PRUSS) += pruss.o obj-$(CONFIG_MFD_DM355EVM_MSP) += dm355evm_msp.o obj-$(CONFIG_MFD_TI_SSP) += ti-ssp.o diff --git a/drivers/mfd/pruss.c b/drivers/mfd/pruss.c new file mode 100644 index 0000000..6836d5a --- /dev/null +++ b/drivers/mfd/pruss.c @@ -0,0 +1,513 @@ +/* + * Copyright (C) 2010, 2011 Texas Instruments Incorporated + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation version 2. + * + * This program is distributed "as is" WITHOUT ANY WARRANTY of any kind, + * whether express or implied; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/types.h> +#include <linux/module.h> +#include <linux/bitops.h> +#include <linux/interrupt.h> +#include <linux/spinlock.h> +#include <linux/errno.h> +#include <linux/platform_device.h> +#include <linux/clk.h> +#include <linux/mfd/pruss.h> +#include <linux/mfd/core.h> +#include <linux/io.h> +#include <linux/slab.h> +#include <linux/err.h> + +struct pruss_priv { + struct device *dev; + spinlock_t lock; + struct resource *res; + struct clk *clk; + void __iomem *ioaddr; +}; + +s32 pruss_disable(struct device *dev, u8 pruss_num) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + struct prusscore_regs __iomem *h_pruss; + struct pruss_map __iomem *pruss_mmap = pruss->ioaddr; + u32 temp_reg; + + if ((pruss_num != PRUCORE_0) && (pruss_num != PRUCORE_1)) + return -EINVAL; + + spin_lock(&pruss->lock); + + /* pruss deinit */ + iowrite32(0xFFFFFFFF, &pruss_mmap->intc.statclrint[pruss_num]); + + /* Disable PRU */ + h_pruss = &pruss_mmap->core[pruss_num]; + temp_reg = ioread32(&h_pruss->control); + temp_reg = (temp_reg & + ~PRUCORE_CONTROL_COUNTENABLE_MASK) | + ((PRUCORE_CONTROL_COUNTENABLE_DISABLE << + PRUCORE_CONTROL_COUNTENABLE_SHIFT) & + PRUCORE_CONTROL_COUNTENABLE_MASK); + iowrite32(temp_reg, &h_pruss->control); + + temp_reg = ioread32(&h_pruss->control); + temp_reg = (temp_reg & + ~PRUCORE_CONTROL_ENABLE_MASK) | + ((PRUCORE_CONTROL_ENABLE_DISABLE << + PRUCORE_CONTROL_ENABLE_SHIFT) & + PRUCORE_CONTROL_ENABLE_MASK); + iowrite32(temp_reg, &h_pruss->control); + + /* Reset PRU */ + iowrite32(PRUCORE_CONTROL_RESETVAL, + &h_pruss->control); + spin_unlock(&pruss->lock); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_disable); + +s32 pruss_enable(struct device *dev, u8 pruss_num) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + struct prusscore_regs __iomem *h_pruss; + struct pruss_map __iomem *pruss_mmap = pruss->ioaddr; + u32 i; + + if ((pruss_num != PRUCORE_0) && (pruss_num != PRUCORE_1)) + return -EINVAL; + + h_pruss = &pruss_mmap->core[pruss_num]; + + /* Reset PRU */ + spin_lock(&pruss->lock); + iowrite32(PRUCORE_CONTROL_RESETVAL, &h_pruss->control); + spin_unlock(&pruss->lock); + + /* Reset any garbage in the ram */ + if (pruss_num == PRUCORE_0) + for (i = 0; i < PRUSS_PRU0_RAM_SZ; i++) + iowrite32(0x0, &pruss_mmap->dram0[i]); + else if (pruss_num == PRUCORE_1) + for (i = 0; i < PRUSS_PRU1_RAM_SZ; i++) + iowrite32(0x0, &pruss_mmap->dram1[i]); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_enable); + +/* Load the specified PRU with code */ +s32 pruss_load(struct device *dev, u8 pruss_num, + u32 *pruss_code, u32 code_size_in_words) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + struct pruss_map __iomem *pruss_mmap = pruss->ioaddr; + u32 __iomem *pruss_iram; + u32 i; + + if (pruss_num == PRUCORE_0) + pruss_iram = (u32 __iomem *)&pruss_mmap->iram0; + else if (pruss_num == PRUCORE_1) + pruss_iram = (u32 __iomem *)&pruss_mmap->iram1; + else + return -EINVAL; + + pruss_enable(dev, pruss_num); + + spin_lock(&pruss->lock); + /* Copy dMAX code to its instruction RAM */ + for (i = 0; i < code_size_in_words; i++) + iowrite32(pruss_code[i], (pruss_iram + i)); + + spin_unlock(&pruss->lock); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_load); + +s32 pruss_run(struct device *dev, u8 pruss_num) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + struct prusscore_regs __iomem *h_pruss; + struct pruss_map __iomem *pruss_mmap = pruss->ioaddr; + u32 temp_reg; + + if ((pruss_num != PRUCORE_0) && (pruss_num != PRUCORE_1)) + return -EINVAL; + + h_pruss = &pruss_mmap->core[pruss_num]; + + /* Enable dMAX, let it execute the code we just copied */ + spin_lock(&pruss->lock); + temp_reg = ioread32(&h_pruss->control); + temp_reg = (temp_reg & + ~PRUCORE_CONTROL_COUNTENABLE_MASK) | + ((PRUCORE_CONTROL_COUNTENABLE_ENABLE << + PRUCORE_CONTROL_COUNTENABLE_SHIFT) & + PRUCORE_CONTROL_COUNTENABLE_MASK); + iowrite32(temp_reg, &h_pruss->control); + + temp_reg = ioread32(&h_pruss->control); + temp_reg = (temp_reg & + ~PRUCORE_CONTROL_ENABLE_MASK) | + ((PRUCORE_CONTROL_ENABLE_ENABLE << + PRUCORE_CONTROL_ENABLE_SHIFT) & + PRUCORE_CONTROL_ENABLE_MASK); + iowrite32(temp_reg, &h_pruss->control); + spin_unlock(&pruss->lock); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_run); + +s32 pruss_wait_for_halt(struct device *dev, u8 pruss_num, u32 timeout) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + struct prusscore_regs __iomem *h_pruss; + struct pruss_map __iomem *pruss_mmap = pruss->ioaddr; + u32 temp_reg; + u32 cnt = timeout; + + if ((pruss_num != PRUCORE_0) && (pruss_num != PRUCORE_1)) + return -EINVAL; + + h_pruss = &pruss_mmap->core[pruss_num]; + + while (cnt--) { + temp_reg = ioread32(&h_pruss->control); + if (((temp_reg & PRUCORE_CONTROL_RUNSTATE_MASK) >> + PRUCORE_CONTROL_RUNSTATE_SHIFT) == + PRUCORE_CONTROL_RUNSTATE_HALT) + break; + } + if (!cnt) + return -EBUSY; + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_wait_for_halt); + +s32 pruss_writeb(struct device *dev, u32 offset, u8 pdatatowrite) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + void __iomem *paddresstowrite; + + paddresstowrite = pruss->ioaddr + offset; + iowrite8(pdatatowrite, paddresstowrite); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_writeb); + +s32 pruss_rmwb(struct device *dev, u32 offset, u8 mask, u8 val) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + void __iomem *paddress; + u32 preg_data; + + paddress = pruss->ioaddr + offset; + + spin_lock(&pruss->lock); + preg_data = ioread8(paddress); + preg_data &= ~mask; + preg_data |= val; + iowrite8(preg_data, paddress); + spin_unlock(&pruss->lock); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_rmwb); + +s32 pruss_readb(struct device *dev, u32 offset, u8 *pdatatoread) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + void __iomem *paddresstoread; + + paddresstoread = pruss->ioaddr + offset ; + *pdatatoread = ioread8(paddresstoread); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_readb); + +s32 pruss_readb_multi(struct device *dev, u32 offset, + u8 *pdatatoread, u16 bytestoread) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + u8 __iomem *paddresstoread; + u16 i; + + paddresstoread = pruss->ioaddr + offset; + + for (i = 0; i < bytestoread; i++) + *pdatatoread++ = ioread8(paddresstoread++); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_readb_multi); + +s32 pruss_writel(struct device *dev, u32 offset, + u32 pdatatowrite) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + void __iomem *paddresstowrite; + + paddresstowrite = pruss->ioaddr + offset; + iowrite32(pdatatowrite, paddresstowrite); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_writel); + +s32 pruss_writel_multi(struct device *dev, u32 offset, + u32 *pdatatowrite, u16 wordstowrite) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + u32 __iomem *paddresstowrite; + u16 i; + + paddresstowrite = pruss->ioaddr + offset; + + for (i = 0; i < wordstowrite; i++) + iowrite32(*pdatatowrite++, paddresstowrite++); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_writel_multi); + +s32 pruss_rmwl(struct device *dev, u32 offset, u32 mask, u32 val) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + void __iomem *paddress; + u32 preg_data; + + paddress = pruss->ioaddr + offset; + + spin_lock(&pruss->lock); + preg_data = ioread32(paddress); + preg_data &= ~mask; + preg_data |= val; + iowrite32(preg_data, paddress); + spin_unlock(&pruss->lock); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_rmwl); + +s32 pruss_readl(struct device *dev, u32 offset, u32 *pdatatoread) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + void __iomem *paddresstoread; + + paddresstoread = pruss->ioaddr + offset; + *pdatatoread = ioread32(paddresstoread); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_readl); + +s32 pruss_readl_multi(struct device *dev, u32 offset, + u32 *pdatatoread, u16 wordstoread) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + u32 __iomem *paddresstoread; + u16 i; + + paddresstoread = pruss->ioaddr + offset; + for (i = 0; i < wordstoread; i++) + *pdatatoread++ = ioread32(paddresstoread++); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_readl_multi); + +s32 pruss_writew(struct device *dev, u32 offset, u16 pdatatowrite) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + void __iomem *paddresstowrite; + + paddresstowrite = pruss->ioaddr + offset; + iowrite16(pdatatowrite, paddresstowrite); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_writew); + +s32 pruss_rmww(struct device *dev, u32 offset, u16 mask, u16 val) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + void __iomem *paddress; + u32 preg_data; + + paddress = pruss->ioaddr + offset; + + spin_lock(&pruss->lock); + preg_data = ioread16(paddress); + preg_data &= ~mask; + preg_data |= val; + iowrite16(preg_data, paddress); + spin_unlock(&pruss->lock); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_rmww); + +s32 pruss_readw(struct device *dev, u32 offset, u16 *pdatatoread) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + void __iomem *paddresstoread; + + paddresstoread = pruss->ioaddr + offset; + *pdatatoread = ioread16(paddresstoread); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_readw); + +s32 pruss_idx_writel(struct device *dev, u32 offset, u32 value) +{ + struct pruss_priv *pruss = dev_get_drvdata(dev->parent); + void __iomem *paddresstowrite; + + paddresstowrite = pruss->ioaddr + offset; + iowrite32(value, paddresstowrite); + + return 0; +} +EXPORT_SYMBOL_GPL(pruss_idx_writel); + +static int pruss_mfd_add_devices(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct mfd_cell *cell = pdev->dev.platform_data; + s32 err, i, num_devices = 0; + + for (i = 0; cell[i].name; i++) { + err = mfd_add_devices(dev, 0, &cell[i], 1, NULL, 0); + if (err) { + dev_err(dev, "cannot add mfd cell: %s\n", + cell[i].name); + continue; + } + num_devices++; + dev_info(dev, "mfd: added %s device\n", cell[i].name); + } + + return num_devices; +} + +static int __devinit pruss_probe(struct platform_device *pdev) +{ + struct pruss_priv *pruss_dev = NULL; + s32 err; + + pruss_dev = kzalloc(sizeof(struct pruss_priv), GFP_KERNEL); + if (!pruss_dev) + return -ENOMEM; + + pruss_dev->res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!pruss_dev->res) { + dev_err(&pdev->dev, + "unable to get pruss memory resources!\n"); + err = -ENODEV; + goto probe_exit_kfree; + } + + if (!request_mem_region(pruss_dev->res->start, + resource_size(pruss_dev->res), dev_name(&pdev->dev))) { + dev_err(&pdev->dev, "pruss memory region already claimed!\n"); + err = -EBUSY; + goto probe_exit_kfree; + } + + pruss_dev->ioaddr = ioremap(pruss_dev->res->start, + resource_size(pruss_dev->res)); + if (!pruss_dev->ioaddr) { + dev_err(&pdev->dev, "ioremap failed\n"); + err = -ENOMEM; + goto probe_exit_free_region; + } + + pruss_dev->clk = clk_get(NULL, "pruss"); + if (IS_ERR(pruss_dev->clk)) { + dev_err(&pdev->dev, "no clock available: pruss\n"); + err = -ENODEV; + pruss_dev->clk = NULL; + goto probe_exit_iounmap; + } + spin_lock_init(&pruss_dev->lock); + + clk_enable(pruss_dev->clk); + + err = pruss_mfd_add_devices(pdev); + if (!err) + goto probe_exit_clock; + + platform_set_drvdata(pdev, pruss_dev); + pruss_dev->dev = &pdev->dev; + return 0; + +probe_exit_clock: + clk_put(pruss_dev->clk); + clk_disable(pruss_dev->clk); +probe_exit_iounmap: + iounmap(pruss_dev->ioaddr); +probe_exit_free_region: + release_mem_region(pruss_dev->res->start, + resource_size(pruss_dev->res)); +probe_exit_kfree: + kfree(pruss_dev); + return err; +} + +static int __devexit pruss_remove(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct pruss_priv *pruss = dev_get_drvdata(dev); + + mfd_remove_devices(dev); + pruss_disable(dev, PRUCORE_0); + pruss_disable(dev, PRUCORE_1); + clk_disable(pruss->clk); + clk_put(pruss->clk); + iounmap(pruss->ioaddr); + release_mem_region(pruss->res->start, resource_size(pruss->res)); + kfree(pruss); + dev_set_drvdata(dev, NULL); + return 0; +} + +static struct platform_driver pruss_driver = { + .probe = pruss_probe, + .remove = __devexit_p(pruss_remove), + .driver = { + .name = "pruss_mfd", + .owner = THIS_MODULE, + } +}; + +static int __init pruss_init(void) +{ + return platform_driver_register(&pruss_driver); +} +module_init(pruss_init); + +static void __exit pruss_exit(void) +{ + platform_driver_unregister(&pruss_driver); +} +module_exit(pruss_exit); + +MODULE_DESCRIPTION("Programmable Realtime Unit (PRU) Driver"); +MODULE_AUTHOR("Subhasish Ghosh"); +MODULE_LICENSE("GPL"); diff --git a/include/linux/mfd/pruss.h b/include/linux/mfd/pruss.h new file mode 100644 index 0000000..8ef25b3 --- /dev/null +++ b/include/linux/mfd/pruss.h @@ -0,0 +1,130 @@ +/* + * Copyright (C) 2010, 2011 Texas Instruments Incorporated + * Author: Jitendra Kumar <jitendra@mistralsolutions.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation version 2. + * + * This program is distributed "as is" WITHOUT ANY WARRANTY of any kind, + * whether express or implied; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#ifndef _PRUSS_H_ +#define _PRUSS_H_ + +#include <linux/types.h> +#include <linux/platform_device.h> +#include "pruss_core.h" + +#define PRUSS_NUM0 PRUCORE_0 +#define PRUSS_NUM1 PRUCORE_1 + +#define PRUSS_PRU0_RAM_SZ 512 +#define PRUSS_PRU1_RAM_SZ 512 +#define PRUSS_PRU0_BASE_ADDRESS 0 +#define PRUSS_PRU1_BASE_ADDRESS 0x2000 +#define PRUSS_INTC_BASE_ADDRESS (PRUSS_PRU0_BASE_ADDRESS + 0x4000) +#define PRUSS_INTC_GLBLEN (PRUSS_INTC_BASE_ADDRESS + 0x10) +#define PRUSS_INTC_GLBLNSTLVL (PRUSS_INTC_BASE_ADDRESS + 0x1C) +#define PRUSS_INTC_STATIDXSET (PRUSS_INTC_BASE_ADDRESS + 0x20) +#define PRUSS_INTC_STATIDXCLR (PRUSS_INTC_BASE_ADDRESS + 0x24) +#define PRUSS_INTC_ENIDXSET (PRUSS_INTC_BASE_ADDRESS + 0x28) +#define PRUSS_INTC_ENIDXCLR (PRUSS_INTC_BASE_ADDRESS + 0x2C) +#define PRUSS_INTC_HSTINTENIDXSET (PRUSS_INTC_BASE_ADDRESS + 0x34) +#define PRUSS_INTC_HSTINTENIDXCLR (PRUSS_INTC_BASE_ADDRESS + 0x38) +#define PRUSS_INTC_GLBLPRIIDX (PRUSS_INTC_BASE_ADDRESS + 0x80) +#define PRUSS_INTC_STATSETINT0 (PRUSS_INTC_BASE_ADDRESS + 0x200) +#define PRUSS_INTC_STATSETINT1 (PRUSS_INTC_BASE_ADDRESS + 0x204) +#define PRUSS_INTC_STATCLRINT0 (PRUSS_INTC_BASE_ADDRESS + 0x280) +#define PRUSS_INTC_STATCLRINT1 (PRUSS_INTC_BASE_ADDRESS + 0x284) +#define PRUSS_INTC_ENABLESET0 (PRUSS_INTC_BASE_ADDRESS + 0x300) +#define PRUSS_INTC_ENABLESET1 (PRUSS_INTC_BASE_ADDRESS + 0x304) +#define PRUSS_INTC_ENABLECLR0 (PRUSS_INTC_BASE_ADDRESS + 0x380) +#define PRUSS_INTC_ENABLECLR1 (PRUSS_INTC_BASE_ADDRESS + 0x384) +#define PRUSS_INTC_CHANMAP0 (PRUSS_INTC_BASE_ADDRESS + 0x400) +#define PRUSS_INTC_CHANMAP1 (PRUSS_INTC_BASE_ADDRESS + 0x404) +#define PRUSS_INTC_CHANMAP2 (PRUSS_INTC_BASE_ADDRESS + 0x408) +#define PRUSS_INTC_CHANMAP3 (PRUSS_INTC_BASE_ADDRESS + 0x40C) +#define PRUSS_INTC_CHANMAP4 (PRUSS_INTC_BASE_ADDRESS + 0x410) +#define PRUSS_INTC_CHANMAP5 (PRUSS_INTC_BASE_ADDRESS + 0x414) +#define PRUSS_INTC_CHANMAP6 (PRUSS_INTC_BASE_ADDRESS + 0x418) +#define PRUSS_INTC_CHANMAP7 (PRUSS_INTC_BASE_ADDRESS + 0x41C) +#define PRUSS_INTC_CHANMAP8 (PRUSS_INTC_BASE_ADDRESS + 0x420) +#define PRUSS_INTC_CHANMAP9 (PRUSS_INTC_BASE_ADDRESS + 0x424) +#define PRUSS_INTC_CHANMAP10 (PRUSS_INTC_BASE_ADDRESS + 0x428) +#define PRUSS_INTC_CHANMAP11 (PRUSS_INTC_BASE_ADDRESS + 0x42C) +#define PRUSS_INTC_CHANMAP12 (PRUSS_INTC_BASE_ADDRESS + 0x430) +#define PRUSS_INTC_CHANMAP13 (PRUSS_INTC_BASE_ADDRESS + 0x434) +#define PRUSS_INTC_CHANMAP14 (PRUSS_INTC_BASE_ADDRESS + 0x438) +#define PRUSS_INTC_CHANMAP15 (PRUSS_INTC_BASE_ADDRESS + 0x43C) +#define PRUSS_INTC_HOSTMAP0 (PRUSS_INTC_BASE_ADDRESS + 0x800) +#define PRUSS_INTC_HOSTMAP1 (PRUSS_INTC_BASE_ADDRESS + 0x804) +#define PRUSS_INTC_HOSTMAP2 (PRUSS_INTC_BASE_ADDRESS + 0x808) +#define PRUSS_INTC_POLARITY0 (PRUSS_INTC_BASE_ADDRESS + 0xD00) +#define PRUSS_INTC_POLARITY1 (PRUSS_INTC_BASE_ADDRESS + 0xD04) +#define PRUSS_INTC_TYPE0 (PRUSS_INTC_BASE_ADDRESS + 0xD80) +#define PRUSS_INTC_TYPE1 (PRUSS_INTC_BASE_ADDRESS + 0xD84) +#define PRUSS_INTC_HOSTINTEN (PRUSS_INTC_BASE_ADDRESS + 0x1500) +#define PRUSS_INTC_HOSTINTLVL_MAX 9 + +#define PRU_INTC_HOSTMAP0_CHAN (0x03020100) +#define PRU_INTC_HOSTMAP1_CHAN (0x07060504) +#define PRU_INTC_HOSTMAP2_CHAN (0x00000908) + +#define PRU_INTC_CHANMAP7_SYS_EVT31 (0x00000000) +#define PRU_INTC_CHANMAP8_FULL (0x02020100) +#define PRU_INTC_CHANMAP9_FULL (0x04040303) +#define PRU_INTC_CHANMAP10_FULL (0x06060505) +#define PRU_INTC_CHANMAP11_FULL (0x08080707) +#define PRU_INTC_CHANMAP12_FULL (0x00010909) +#define PRU_INTC_CHANMAP8_HALF (0x03020100) +#define PRU_INTC_CHANMAP9_HALF (0x07060504) +#define PRU_INTC_CHANMAP10_HALF (0x03020908) +#define PRU_INTC_CHANMAP11_HALF (0x07060504) +#define PRU_INTC_CHANMAP12_HALF (0x00010908) +#define PRU_INTC_REGMAP_MASK (0xFFFFFFFF) + +s32 pruss_enable(struct device *dev, u8 pruss_num); + +s32 pruss_load(struct device *dev, u8 pruss_num, + u32 *pruss_code, u32 code_size_in_words); + +s32 pruss_run(struct device *dev, u8 pruss_num); + +s32 pruss_wait_for_halt(struct device *dev, u8 pruss_num, u32 timeout); + +s32 pruss_disable(struct device *dev, u8 pruss_num); + +s32 pruss_writeb(struct device *dev, u32 offset, u8 pdatatowrite); + +s32 pruss_rmwb(struct device *dev, u32 offset, u8 mask, u8 val); + +s32 pruss_readb(struct device *dev, u32 offset, u8 *pdatatoread); + +s32 pruss_readb_multi(struct device *dev, u32 offset, + u8 *pdatatoread, u16 bytestoread); + +s32 pruss_readl(struct device *dev, u32 offset, u32 *pdatatoread); + +s32 pruss_readl_multi(struct device *dev, u32 offset, + u32 *pdatatoread, u16 wordstoread); + +s32 pruss_writel(struct device *dev, u32 offset, u32 pdatatowrite); + +s32 pruss_writel_multi(struct device *dev, u32 offset, + u32 *pdatatowrite, u16 wordstowrite); + +s32 pruss_rmwl(struct device *dev, u32 offset, u32 mask, u32 val); + +s32 pruss_idx_writel(struct device *dev, u32 offset, u32 value); + +s32 pruss_writew(struct device *dev, u32 offset, u16 datatowrite); + +s32 pruss_rmww(struct device *dev, u32 offset, u16 mask, u16 val); + +s32 pruss_readw(struct device *dev, u32 offset, u16 *pdatatoread); + +#endif /* End _PRUSS_H_ */ diff --git a/include/linux/mfd/pruss_core.h b/include/linux/mfd/pruss_core.h new file mode 100644 index 0000000..48e2b99 --- /dev/null +++ b/include/linux/mfd/pruss_core.h @@ -0,0 +1,128 @@ +/* + * Copyright (C) 2010, 2011 Texas Instruments Incorporated + * Author: Jitendra Kumar <jitendra@mistralsolutions.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation version 2. + * + * This program is distributed "as is" WITHOUT ANY WARRANTY of any kind, + * whether express or implied; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#ifndef _PRUSS_CORE_H_ +#define _PRUSS_CORE_H_ + +#include <linux/types.h> + +#define PRUCORE_0 (0) +#define PRUCORE_1 (1) + +#define PRUCORE_CONTROL_PCRESETVAL_MASK (0xFFFF0000u) +#define PRUCORE_CONTROL_PCRESETVAL_SHIFT (0x00000010u) +#define PRUCORE_CONTROL_PCRESETVAL_RESETVAL (0x00000000u) +#define PRUCORE_CONTROL_RUNSTATE_MASK (0x00008000u) +#define PRUCORE_CONTROL_RUNSTATE_SHIFT (0x0000000Fu) +#define PRUCORE_CONTROL_RUNSTATE_RESETVAL (0x00000000u) +#define PRUCORE_CONTROL_RUNSTATE_HALT (0x00000000u) +#define PRUCORE_CONTROL_RUNSTATE_RUN (0x00000001u) +#define PRUCORE_CONTROL_SINGLESTEP_MASK (0x00000100u) +#define PRUCORE_CONTROL_SINGLESTEP_SHIFT (0x00000008u) +#define PRUCORE_CONTROL_SINGLESTEP_RESETVAL (0x00000000u) +#define PRUCORE_CONTROL_SINGLESTEP_FREERUN (0x00000000u) +#define PRUCORE_CONTROL_SINGLESTEP_SINGLE (0x00000001u) +#define PRUCORE_CONTROL_COUNTENABLE_MASK (0x00000008u) +#define PRUCORE_CONTROL_COUNTENABLE_SHIFT (0x00000003u) +#define PRUCORE_CONTROL_COUNTENABLE_RESETVAL (0x00000000u) +#define PRUCORE_CONTROL_COUNTENABLE_DISABLE (0x00000000u) +#define PRUCORE_CONTROL_COUNTENABLE_ENABLE (0x00000001u) +#define PRUCORE_CONTROL_SLEEPING_MASK (0x00000004u) +#define PRUCORE_CONTROL_SLEEPING_SHIFT (0x00000002u) +#define PRUCORE_CONTROL_SLEEPING_RESETVAL (0x00000000u) +#define PRUCORE_CONTROL_SLEEPING_NOTASLEEP (0x00000000u) +#define PRUCORE_CONTROL_SLEEPING_ASLEEP (0x00000001u) +#define PRUCORE_CONTROL_ENABLE_MASK (0x00000002u) +#define PRUCORE_CONTROL_ENABLE_SHIFT (0x00000001u) +#define PRUCORE_CONTROL_ENABLE_RESETVAL (0x00000000u) +#define PRUCORE_CONTROL_ENABLE_DISABLE (0x00000000u) +#define PRUCORE_CONTROL_ENABLE_ENABLE (0x00000001u) +#define PRUCORE_CONTROL_SOFTRESET_MASK (0x00000001u) +#define PRUCORE_CONTROL_SOFTRESET_SHIFT (0x00000000u) +#define PRUCORE_CONTROL_SOFTRESET_RESETVAL (0x00000000u) +#define PRUCORE_CONTROL_SOFTRESET_RESET (0x00000000u) +#define PRUCORE_CONTROL_SOFTRESET_OUT_OF_RESET (0x00000001u) +#define PRUCORE_CONTROL_RESETVAL (0x00000000u) + +struct prusscore_regs { + u32 control; + u32 status; + u32 wakeup; + u32 cyclecnt; + u32 stallcnt; + u8 rsvd0[12]; + u32 contabblkidx0; + u32 contabblkidx1; + u32 contabproptr0; + u32 contabproptr1; + u8 rsvd1[976]; + u32 intgpr[32]; + u32 intcter[32]; + u8 rsvd2[768]; +}; + +struct pruss_intc_regs { + u32 revid; + u32 control; + u8 res1[8]; + u32 glblen; + u8 res2[8]; + u32 glblnstlvl; + u32 statidxset; + u32 statidxclr; + u32 enidxset; + u32 enidxclr; + u8 res3[4]; + u32 hostintenidxset; + u32 hostintenidxclr; + u8 res4[68]; + u32 glblpriidx; + u8 res5[380]; + u32 statsetint[2]; + u8 res6[120]; + u32 statclrint[2]; + u8 res7[120]; + u32 enableset[2]; + u8 res8[120]; + u32 enableclr[2]; + u8 res9[120]; + u32 chanmap[16]; + u8 res10[960]; + u32 hostmap[2]; + u8 res11[248]; + u32 hostintpriidx[10]; + u8 res12[984]; + u32 polarity[2]; + u8 res13[120]; + u32 type[2]; + u8 res14[888]; + u32 hostintnstlvl[10]; + u8 res15[984]; + u32 hostinten; + u8 res16[6907]; +}; + +struct pruss_map { + u8 dram0[512]; + u8 res1[7680]; + u8 dram1[512]; + u8 res2[7680]; + struct pruss_intc_regs intc; + struct prusscore_regs core[2]; + u8 iram0[4096]; + u8 res3[12288]; + u8 iram1[4096]; + u8 res4[12288]; +}; +#endif
This patch adds the pruss MFD driver and associated include files. For details regarding the PRUSS please refer the folowing link: http://processors.wiki.ti.com/index.php/Programmable_Realtime_Unit_Subsystem The rational behind the MFD driver being the fact that multiple devices can be implemented on the cores independently. This is determined by the nature of the program which is loaded into the PRU's instruction memory. A device may be de-initialized and another loaded or two different devices can be run simultaneously on the two cores. It's also possible, as in our case, to implement a single device on both the PRU's resulting in improved load sharing. Signed-off-by: Subhasish Ghosh <subhasish@mistralsolutions.com> --- drivers/mfd/Kconfig | 10 + drivers/mfd/Makefile | 1 + drivers/mfd/pruss.c | 513 ++++++++++++++++++++++++++++++++++++++++ include/linux/mfd/pruss.h | 130 ++++++++++ include/linux/mfd/pruss_core.h | 128 ++++++++++ 5 files changed, 782 insertions(+), 0 deletions(-) create mode 100644 drivers/mfd/pruss.c create mode 100644 include/linux/mfd/pruss.h create mode 100644 include/linux/mfd/pruss_core.h