daniel-santos / mcp2210-linux

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

Added support for Device Tree based configuration #27

Open ronakadesai opened 7 years ago

ronakadesai commented 7 years ago

This patch has been added to make this driver configurable through Linux device tree.

Notes : This changes have been tested on a ppc target hardware.

Signed-off-by: Ronak Desai ronak.desai@rockwellcollins.com

daniel-santos commented 7 years ago

I'm hoping to have time to go through this tomorrow. Keep in mind that the ioctl interface must eventually go since it's been deprecated in favor of netlink. I created Issue #2 for that some time back.

daniel-santos commented 7 years ago

I've had a little time to review it today. The first thing is that I don't want this tacked on to the ioctl code -- that's hacky. I would rather the common part of mcp2210_ioctl_config_set() used by both ioctl and device tree code be abstracted and moved somewhere else -- keep the ioctl functions & structures private. This promotes loose coupling and good encapsulation.

As far as the bigger picture, I have a few concerns and maybe lack of understanding, since I haven't used device tree yet. When you call of_find_compatible_node, where does it retrieve the data from? Is this device tree data compiled into the kernel? And if so, does this feature mean that it only expects one type of USB device with an MCP2210 chip to be connected? I don't have a problem with that, but I want to make sure that I understand. Thanks

dhilst commented 7 years ago

Hi, I've tried to aply this patch to master but I got problems at compile time.

The compiler is complaining about completion member of struct ioctl_result @ mcp2210.h while compiling mcp2210-lib.c. Is this struct passed to userspace at all? I see that it was moved from mcp2210-ioctl.c to mc2210.h by this pull request. To get it compiled I surround this ones with #if defined(KERNEL) #endif

Regards,

dhilst commented 7 years ago

I'm trying to add my device at the bus created by mcp2210.ko but my driver can't find it's device-tree configuration. Here is a gist with the configuration that I'm using and a test case :) https://gist.github.com/gkos/b54054b345afc4239e92f8adde99788e

daniel-santos commented 7 years ago

Well, I'm sorry for being so late to reply to you gkos. I won't be using this patch as-is, but I definitely want this functionality, and will likely use this patch as a basis for it. onakadesai has done the bulk of the work to make this happen. :)

dhilst commented 7 years ago

Okay, no problem man! 😃