[PATCH V2] ST SPEAr: PCIE gadget suppport
pratyush.anand at st.com
Mon Jan 10 06:20:36 EST 2011
Thanks for the comments!!!
On 1/8/2011 4:02 AM, Arnd Bergmann wrote:
> On Thursday 06 January 2011, Viresh Kumar wrote:
>> From: Pratyush Anand <pratyush.anand at st.com>
>> This is a configurable gadget. can be configured by sysfs interface. Any
>> IP available at PCIE bus can be programmed to be used by host
>> controller.It supoorts both INTX and MSI.
>> By default, gadget is configured for INTX and SYSRAM1 is mapped to BAR0
>> with size 0x1000
> The code looks reasonably clean, but I don't yet understand how you would
> use this device for the interesting case of communicating with the host
> side. I've put a lot of thought into similar hardware designs before, but
> never got an implementation done myself.
This driver has been developed just to show spear's capability to work as dual
mode pcie controller.
> Forwarding devices that don't need interrupts with your driver seems to
> be the only case that will easily work, but that is also rather limited
> in what you can use it for.
Off-course, it will work best with non interrupting devices. One can use
INTA assertion/de-assertion by software, but it would be very slow.
> One concept that people have played with before is to export a virtio
> channel through PCI that you can use with e.g. virtio-net or virtfs.
> This is very powerful and has a lot of possible applications because
> we have support for virtio drivers across a number of operating systems
> and platforms already.
>> Documentation/misc-devices/spear-pcie-gadget.txt | 125 ++++
>> drivers/misc/Kconfig | 10 +
>> drivers/misc/Makefile | 1 +
>> drivers/misc/spear13xx_pcie_gadget.c | 856 ++++++++++++++++++++++
> Why would you put this under drivers/misc?
> I think drivers/pci/gadget/ would be a better place. I would also
> recommend splitting the user interface from the hardware dependent
> parts, so someone else can easily do another gadget driver for
> different hardware.
Yes, can be done, if the interfaces which I have made is
acceptable to community.
>> +/*wait till link is up*/
>> +# cat sys/devices/platform/pcie-gadget-spear.0/link
>> +wait till it returns UP.
> A blocking sysfs read is not a nice interface. This is probably where
> the sysfs abstraction for your hardware stops making sense.
This call is not blocking. User will have to recheck link status till he
finds it UP. He may put some delay between two successive read. I will
modify documentation to be more explicit.
>> +To assert INTA
>> +# echo 1 >> sys/devices/platform/pcie-gadget-spear.0/inta
>> +To de-assert INTA
>> +# echo 0 >> sys/devices/platform/pcie-gadget-spear.0/inta
> And this looks somewhat impractical and slow.
Yes, it will be slow.
>> +to send msi vector 2
>> +# echo 2 >> sys/devices/platform/pcie-gadget-spear.0/send_msi
> Using a sysfs file only for its side-effect is not very nice either.
> The user interface for the interrupts looks to me like it should really
> be based around a character device and either read/write/poll or
> ioctl and poll. Using an eventfd might be cool here, because then you
> can combine this with other devices by passing the event file to
> an interface that operates on eventfd. This would e.g. make it possible
> to combine a UIO device generating interrupts with a PCIe gadget
> sending the interrupts somewhere else, without leaving kernel
I do not have much idea about eventfd mechanism. But if we decide to
split it in two layers (generic pcie gadget and HW specific) then I
might try to do it in this way.
> For setting up the basic parameters, sysfs (or configfs, as Greg suggested)
> is a reasonable choice, so there is no need to change that.
> I don't know what the rules for configfs are though, i.e. if eventfd
> files or ioctls are ok or not.
More information about the linux-arm-kernel