Skip to content
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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Misc updates #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3722466
Update for linux 6.8
mjsir911 Mar 9, 2024
1c8284b
Add GPIO names
mjsir911 Mar 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions gpio-ch341.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,15 @@ static int ch341_gpio_probe(struct platform_device *pdev)
gpio->set_multiple = ch341_gpio_set_multiple;
gpio->base = -1;
gpio->ngpio = CH341_GPIO_NUM_PINS;
gpio->names = (char const * const []){
"D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7",
// names are from "parallel" mode, seems to be what datasheets us
"ERR#", "PEMP", "INT#", "SLCT", "RST#", "WAIT#", "DS#", "AS#",
// names are from "print port" mode, seems to be what datasheets us
// "ERR#", "PEMP", "ACK#", "SLCT", "INI#", "BUSY", "AFD#", "SIN#",

// No WR#?
};
Copy link
Owner

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.

Copy link
Author

@mjsir911 mjsir911 Mar 13, 2024 •
edited
Loading

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);

gpio->can_sleep = true;

girq = &gpio->irq;
Expand Down
26 changes: 14 additions & 12 deletions spi-ch341.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ struct spi_gpio {
};

struct ch341_spi {
struct spi_master *master;
struct spi_controller *master;
Copy link
Owner

@frank-zago frank-zago Mar 13, 2024 •
edited
Loading

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.

struct mutex spi_lock;
u8 cs_allocated; /* bitmask of allocated CS for SPI */
struct gpio_desc *spi_gpio_core_desc[3];
Expand Down Expand Up @@ -188,12 +188,12 @@ static unsigned int copy_from_device(u8 *rx_buf, const u8 *buf,
}

/* Send a message */
static int ch341_spi_transfer_one_message(struct spi_master *master,
static int ch341_spi_transfer_one_message(struct spi_controller *master,
struct spi_message *m)
{
struct ch341_spi *dev = spi_master_get_devdata(master);
struct ch341_spi *dev = spi_controller_get_devdata(master);
struct spi_device *spi = m->spi;
struct spi_client *client = &dev->spi_clients[spi->chip_select];
struct spi_client *client = &dev->spi_clients[spi_get_chipselect(spi, 0)];
bool lsb = spi->mode & SPI_LSB_FIRST;
struct spi_transfer *xfer;
unsigned int buf_idx = 0;
Expand Down Expand Up @@ -396,8 +396,8 @@ static ssize_t new_device_store(struct device *mdev,
struct device_attribute *attr,
const char *buf, size_t count)
{
struct spi_master *master = container_of(mdev, struct spi_master, dev);
struct ch341_spi *dev = spi_master_get_devdata(master);
struct spi_controller *master = container_of(mdev, struct spi_controller, dev);
struct ch341_spi *dev = spi_controller_get_devdata(master);
struct spi_board_info board_info = {
.mode = SPI_MODE_0,
.max_speed_hz = CH341_SPI_MAX_FREQ,
Expand Down Expand Up @@ -454,8 +454,8 @@ static ssize_t delete_device_store(struct device *mdev,
struct device_attribute *attr,
const char *buf, size_t count)
{
struct spi_master *master = container_of(mdev, struct spi_master, dev);
struct ch341_spi *dev = spi_master_get_devdata(master);
struct spi_controller *master = container_of(mdev, struct spi_controller, dev);
struct ch341_spi *dev = spi_controller_get_devdata(master);
int cs;
int ret;

Expand Down Expand Up @@ -497,22 +497,24 @@ static int match_gpiochip_parent(struct gpio_chip *gc, void *data)
static int ch341_spi_probe(struct platform_device *pdev)
{
struct ch341_ddata *ch341 = dev_get_drvdata(pdev->dev.parent->parent);
struct spi_master *master;
struct spi_controller *master;
struct ch341_spi *dev;
int ret;

master = spi_alloc_master(&pdev->dev, sizeof(*dev));
if (!master)
return -ENOMEM;

dev = spi_master_get_devdata(master);
dev = spi_controller_get_devdata(master);

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)
Copy link
Owner

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().

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See a8ae364 (untested)

Copy link
Author

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

Copy link
Owner

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.

);
if (!dev->gpiochip) {
dev_err(&master->dev, "Parent GPIO chip not found!\n");
ret = -ENODEV;
Expand Down Expand Up @@ -554,7 +556,7 @@ static int ch341_spi_probe(struct platform_device *pdev)
device_remove_file(&dev->master->dev, &dev_attr_new_device);

unreg_master:
spi_master_put(master);
spi_controller_put(master);

return ret;
}
Expand Down