search

daniel-santos / mcp2210-linux

MCP2210 driver for linux
http://danielthesantos.blogspot.com/search/label/mcp2210
51 stars 33 forks source link

Regarding registration with SPI #25

Open ronakadesai opened 8 years ago

ronakadesai commented 8 years ago

Hi,

I am planning to use this driver but I wants to perform pin configuration via device tree and not via this utility as configuration may get change based on hardware.

The best way to do this is I think in the mcp2210 probe where I will parse the device tree node and configure the mcp2210 pins. I saw your note on top that to keep probe function minimal. So, wanted to check with you that will this be a right way or you do have some other thoughts for this?

Second thing, did you ever tried to make this driver device tree based ? If yes, then do you have the source code or starting piece available with you?

CajusH commented 8 years ago

Hi,

I am also interrested in a device tree support. In my case I am trying to use the mcp2210 to connect a display controller with SPI interface. I have a "panel" driver for the display controller for the actual Kernel (4.4.x LTS). In the device tree this panel is below a SPI master, which is missing for the MCP2210 SPI master. Therfore the GPU will not be able to connect to the panel driver.

Best Regards Cajus

ronakadesai commented 8 years ago

I am able to do the spi registration from mcp2210_probe (using static structures) and with that the default spidev is appearing. But, as it's a usb_driver, I am not able to find a way to use device tree with it. I have created mcp2210 node in device tree but I am getting "intf->dev.of_node" as NULL.

ronakadesai commented 8 years ago

I have enabled this driver to make it configurable via device tree. I will soon create a pull request for this.

daniel-santos commented 8 years ago

Very cool! Sorry I haven't been able to keep up with this thread much as I've had a lot going on, but I'm looking forward to it!

ronakadesai commented 8 years ago

Sure, I have submitted a merge request #27. Please review it and provide your feedback. Thanks !

daniel-santos commented 8 years ago

I wouldn't mind making the suggested modifications to the pull request. However, it would help me greatly to get a few questions about the patch answered. Part of this is my lack of experience with DT.

  1. When you call of_find_compatible_node(), where does it retrieve the data from? Is this device tree data compiled into the kernel?
  2. If so, does this feature mean that it only expects one type of USB device with an MCP2210 chip to be connected?

This is my understanding of how the patch set behaves. I don't have a particular problem with that, but I need to make sure that this behavior only occurs when specifically enabled in the config. Also, it would be helpful if you could provide me a sample DT configuration that I can use for testing.

Thank you!

ronakadesai commented 8 years ago

I am sorry but I am tied up with so many things at work so not able to work on this. Answer to your questions,

1) It reads from the device tree blob which gets provided to kernel. 2) There is no limitation on that you can update/modify the device tree node as per your hardware need.

So, for example if you are using a kernel which does not have device tree support then this won't come into the picture and you can still use userspace ioctl to configure it.

In the patch, I have added a Readme file which contains the sample device tree node for mcp2210.

daniel-santos commented 8 years ago

Thank you for taking the time to respond! I totally understand how it gets busy in a project and I appreciate you taking the time to send the pull request.

Sorry for missing that you already provided an example!

In regards to my second question, I think I understand this correctly now, but what I was getting at is that if you provide the device tree blob to the kernel compiled with CONFIG_OF enabled, the driver will then expect that the only MCP2210-based device will be the one described in the device tree blob. Can you please verify that I understand this correctly?

Assuming that that it correct, I think I will merge it with that behavior mostly intact (just rename to CONFIG_MCP2210_OF), but later extend it to allow specifying some other constraints (values of the device name string or EEPROM), as a particular USB-connected device is not (usually) hard-wired to a board. One goal of this driver is to support multiple, heterogeneous MCP2210-based devices that may be connected to and disconnected from the computer over time.

Thanks again for your contribution!

EDIT: I think this also warrants cleaning up and further formalizing the various stages of configuring the device. Unfortunately, the kernel wants to know if we are taking the device or not when we return from probe (and we can't always know at that time), but that's just a minor complication.

ronakadesai commented 8 years ago

Thank you too for creating this massive driver.

I think you misunderstood, device tree can have other nodes as well, you can refer following link you would get the idea. http://lxr.free-electrons.com/source/Documentation/devicetree/

Also, CONFIG_OF you can not rename as it's the standard kernel terminology (open_firmware).

daniel-santos commented 8 years ago

Thank you, I will study this further. And I didn't mean to say rename CONFIG_OF, but to add a separate config value that depended upon CONFIG_OF, but I need to study this further. Thanks!