-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Misc updates #28
base: master
Are you sure you want to change the base?
Misc updates #28
Conversation
WRT to the linux 6.8 renames I could potentially rename more master -> controller if that's desired. Seems in line with what other spi modules do. |
"master" semantics are just for compatibility at this point
// "ERR#", "PEMP", "ACK#", "SLCT", "INI#", "BUSY", "AFD#", "SIN#", | ||
|
||
// No WR#? | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linux doesn't support that when there are 2 or more devices. I had that code a while ago and removed it for that reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as of 6.8.0 it does support it with a warning.:
* Take the names from gc->names and assign them to their GPIO descriptors.
* Warn if a name is already used for a GPIO line on a different GPIO chip.
*
* Note that:
* 1. Non-unique names are still accepted,
* 2. Name collisions within the same GPIO chip are not reported.
But yeah. What other drivers seem to do is append the chip name to it names:
name = devm_kasprintf(gpio->gpio.parent, GFP_KERNEL,
"P%s.%02x", port->name, j);
@@ -59,7 +59,7 @@ struct spi_gpio { | |||
}; | |||
|
|||
struct ch341_spi { | |||
struct spi_master *master; | |||
struct spi_controller *master; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will break the build on older kernels that do not have the new naming.
Add an ifdef for older kernels to define the new names.
|
||
dev->master = master; | ||
dev->ch341 = ch341; | ||
platform_set_drvdata(pdev, dev); | ||
|
||
/* Find the parent's gpiochip */ | ||
dev->gpiochip = gpiochip_find(pdev->dev.parent, match_gpiochip_parent); | ||
dev->gpiochip = gpio_device_get_chip( | ||
gpio_device_find(pdev->dev.parent, match_gpiochip_parent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a similar pacth but never tested it. However gpio_device_find may return an error, which gpio_device_get_chip() may not like.
Also I don't see the required gpio_device_put().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See a8ae364 (untested)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah a8ae364 looks entirely better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you integrate it into your PR? I just don't have time to test it.
No description provided.