Added support for Device Tree based configuration - #27
Conversation
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>
|
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. |
| * of_property_* calls */ | ||
| if (0 == of_property_read_u8(spi_node, "spi,use_cs_gpio", | ||
| &value)) | ||
| board_config.pins[pin].spi.use_cs_gpio = value; |
There was a problem hiding this comment.
I don't remember the reason that's a bitfield, except that it may have originally shared it's byte with some other flags. We can probably just changed it to a u8 and modify any place it's read if it expects a 1 or zero.
|
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 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 |
| } | ||
| #endif /* CONFIG_MCP2210_CREEK */ | ||
|
|
||
| #ifdef CONFIG_OF |
There was a problem hiding this comment.
It would appear that using CONFIG_OF will create a problem for people building the kernel with OF enabled, but not using it in this driver. This should be CONFIG_MCP2210_OF and depend upon CONFIG_OF in Kconfig. Of course, Kconfig isn't actually being used yet, but if nothing else it documents config parameters. As such, we should probably have built-time sanity checks for them in some function that's always compiled, like probe:
BUILD_BUG_ON(IS_ENABLED(CONFIG_MCP2210_OF) && !IS_ENABLED(CONFIG_OF));
BUILD_BUG_ON(IS_ENABLED(CONFIG_MCP2210_OF) && IS_ENABLED(CONFIG_MCP2210_CREEK));
|
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, |
|
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 |
|
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. :) |
|
Okay, no problem man! 😃 |
3 participants
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