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.

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

#INI output signal question #5

Open
Sfinx opened this issue Mar 25, 2023 · 21 comments
Open

#INI output signal question #5

Sfinx opened this issue Mar 25, 2023 · 21 comments
Labels
enhancement New feature or request

Comments

@Sfinx
Copy link

Sfinx commented Mar 25, 2023

How to drive it ? It is 26 output only pin of 341B/A/F, described as #INI "Initialize the printer, active low, connect to INIT" in datasheet in print port mode or as RST# "Reset output, active low" in parallel mode.

@frank-zago
Copy link
Owner

Sorry, no idea. Maybe it's not drivable directly.

@Sfinx
Copy link
Author

Sfinx commented Mar 31, 2023

Looking into lib/x64/ch341_lib.h from their official Linux driver header I guess they have a little bit more GPIO's than 0..15 :

16      RESET#
17      WRITE#
18      SCL (for AUX EEPROM)
19      SDA (for AUX EEPROM)
20      ?
21      ?
22      ?
23      ?

Seems like GPIO16 can control #INI

@frank-zago
Copy link
Owner

Interesting. Their 2023 code exposes more functionality. There is another command (0xa1) that doesn't appear in any doc I know of. It can set that pin and others. I'll need to change the drive to use that, as that appears to be more versatile than the current interface.

@Sfinx
Copy link
Author

Sfinx commented Apr 3, 2023

somebody even wrote small investigation tool for this ;)

https://github.com/nic3-14159/CH347-Research

@frank-zago
Copy link
Owner

I've done a bit of investigation, and yes it is possible to support more GPIOs. 19 in total, including that pin 26. This will be on the back-burner though, as there are a few bugs that recently surfaced.
pins 23 and 24 are those used by i2c, so there's a need to reserve them the same way it is done for spi.

@frank-zago frank-zago added the enhancement New feature or request label May 6, 2023
@Djhg2000
Copy link

I'm looking at replacing a slightly hacked driver for the PineDio USB adapter with this, much more generic, CH341 driver. This is the current driver, for reference: https://github.com/rogerjames99/spi-ch341-usb .

The existing demo code is hardcoded to use that particular driver and unfortunately the INI pin is used in there, so I'll have to rewrite that slightly. I did however find a very informative datasheet and I thought you might find it useful: https://pdf1.alldatasheet.com/datasheet-pdf/view/1132609/ETC2/CH341A.html . According to that one the pins change name depending on the configured mode, so I can imagine putting names into the gpioinfo columns could be a nightmare.

@mr-nice
Copy link
Contributor

mr-nice commented Oct 16, 2023

I'm facing the same problem. I hope to find some time to write a PR. After looking through the other forks for the PineDio I think it's the best to enhance the gpio support and concentrate on this fork rather then creating yet another fork for the PineDino :)

@frank-zago
Copy link
Owner

Sorry. I've slacked a lot on this driver. Still on my todo list, but I've been spending my time elsewhere. What would be important in that change is to not affect the i2c pins with the A1 command. Not sure it's possible though.

These are essentially the notes I had for that command:

0040 a1 6a <DataOut 8-15> <DataOut 0-7> 00 00 00

    filter:
    bit 0 - Datout 8-15 valid
    bit 1 - dirout 8-15 valid
    bit 2 - Dataout 0-7 valid
    bit 3 - dirout 0-7 valid
    bit 4 - DataOut16-32 valid

INI might be the pin 12. Not sure. Experiments are needed.

@mr-nice
Copy link
Contributor

mr-nice commented Oct 18, 2023

Thanks for sharing your notes :) I think INI is bit 16 and pin 26

I wrote a small program with CH34xSetOutput from the vendors library to get more insight about the command. The comment of the function indicates that the command sets all outputs/inputs of the chip. So I think it is not possible to not change the i2c pins. I will need to read the i2c driver code to find out how the code enables those pins.

I have the PineDio and a breakout board of the chip with all pins exposed. So I will be able to connect my logic analyzer to the output pins and check the input pins with a switch or something. The comment of the function also indicates that it could be possible to use more pins as input/output. And also to change active low/high. I will try that in a second test.

My first approach will be to not change anything of the gpio driver logic and just change the write_outputs command with i2c pins enabled, but not exported. I also have a i2c display to test the i2c driver afterwards.

Next step would be to expose all other gpios and change the logic of the driver to support the different pins. And set the gpio pins if the i2c driver is loaded, like the spi driver does. But I think this would be better handled with a second PR. What do you think about that plan?

@mr-nice
Copy link
Contributor

mr-nice commented Oct 18, 2023

I found some time to play with this driver code connected to a logic analyzer which uses a slightly modified version of the write_outputs from the library code with the 0xa1 command.
I can set the outputs without changing the i2c lines at all. So this is not a problem. So I will change the write_outputs as a first step and create a PR for it.

@mr-nice
Copy link
Contributor

mr-nice commented Oct 22, 2023

Hi,
I dived into the topic. I rewrote the driver to support all pins and played with it. But before creating another PR for that I need some advice.

The good thing is, with the new I/O command from the PR above we can drive all gpio lines from 0 to 15 as input or output. The pulseview screenshot shows D7 (l7) D6 (l6) PEMP (l9) DS (l14) INI# (l16) and WR# (17)

pulseview gpio 7 8 9 14 16 17

This works really well so, it is worth changing the command to the new one. I will also test the code with the PineDio later, because I need to rewrite the example to use libgpio instead of sysfs. This would check SPI + gpio.

The bad news is, that using I2C and the gpio lines 16 and 17 results in wrong I2C lines.

I connected a ssd1307 device and run the 3D test of luma. The device still works. But as you can see on this pulseview screenshot. The clock and the data both go to 0. In the case of the ssd1307 this doesn't matter as the clock goes down if any of the (16|17) lines is changed. And after that goes up. As I2C is faster than bitbang gpio we could get away with this wrong behavior. But it feels wrong.

pulseview i2c with gpio 16 17

Possible solutions, don't export SDA and SCL gpio at all and grab gpio 16 and 17 from the I2C driver. But we would need some mechanism to detect that from the gpio driver side to set the bits to ignore pins 16-20. This could be archived with setting the pins to input. The problem with that approach is that the pins are shown wrong as possible input pins in gpioinfo. With that mechanism we could even allow SDA and SCL to be used as output if we claim them and mark them as input. Putting the pins to input would make it easy to check. If all pins from 16-20 are set to input don't set the bit for to enable these lines.

What do you think which direction should we take. Currently I tend to misuse the input/output state as this solves my needs. But it feels wrong for general purpose.

Do you know a better mechanism then misuse the input state?

Another topic on my todo list is to check if we can use the low and high active switch supported by the command.

Thanks,
mr-nice

@mr-nice
Copy link
Contributor

mr-nice commented Oct 22, 2023

After rewriting the PineDio example code to use libgpiod, I can use the PineDio with my modified driver. The modified code can talk to the sx126x chip and the pins also work. Without setting the INI# pin, the busy pin keeps high. With the correct initialization of the device with INI# low -> high. I can send frames. And I get device status information from the chip. So SPI still works with the changed code. I cannot verify that the device actually sends something, as I don't have a second one :D

@frank-zago
Copy link
Owner

Thanks. I'll review that this week.

@Djhg2000
Copy link

Is there a public repo with your forked PineDio example @mr-nice ? I have both the USB version and the PinePhone version so I can give it a test fairly easily.

I was planning to have a go at it myself but I can't figure out how to get gpioinfo to list more than 16 lines even with the latest master of this driver (are the rest only available though libgpiod?).

@mr-nice
Copy link
Contributor

mr-nice commented Nov 11, 2023

@Djhg2000
Hi, sorry for the confusion. The first step was to switch the command and don't add extra GPIO at all. I'm currently working on extending that branch. But I did't find time to rework it.

To take it short this is my branch, with all necessary GPIOs, where I tested the pinedio. It is currently not shape for a PR but is the one I used to test the code. And here you can find my test code for the pinedio. It is based on this example code for the other driver.

Make sure to follow the documentation to export the spi device

I would be glad if you find time to test it :) As I don't own a second device I cannot test that it actually sends anything. Just that the chip setup and the communication looks good.

I hope I'm able to find some time for the PR to extend the command and claim the I2C.

@Djhg2000
Copy link

I gave it a test but the PinePhone (running the demo code from here https://codeberg.org/JF002/pinedio-lora-driver) doesn't seem to receive anything. I've tried sending from the PinePhone too but after looking at the code in your pinedio.cpp I don't see where it's supposed to print received data in the infinite loop.

This is what I get on stdout:

$ ./SX126x_PineDio 
GPIO WRITE << 0 -- 0
write |0|
GPIO WRITE << 0 -- 1
write |1|
read |0|
Status : 34
        Cmd : 1
        Cpu : 0
        Chip : 2
read |0|
Error : 0
read |0|
read |0|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |0|
read |0|
read |0|
read |0|
read |0|
read |0|
read |0|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |1|
read |0|
read |0|
read |0|
read |0|
read |0|
read |0|
read |0|
read |0|
read |0|
read |0|
read |0|
Mode  : 2
Mode  : 2
read |0|
read |0|
read |0|
read |0|
read |0|
0x0 0x1 0x2 0x3 0x4 0x5 0x6 0x7 
read |0|

After a few seconds of silence I get this:

read |0|
Error : 0
read |0|
read |0|
Mode  : 4
read |0|
read |0|
Mode  : 4

The last 3 lines then seem to repeat infinitely. Sending from the PinePhone doesn't change the output.

@Djhg2000
Copy link

I worked my way around the demo code from the JF002 repo and with a few small changes it stared working! Here is the relevant diff:

diff --git a/src/usb-adapter/UsbAdapter.cpp b/src/usb-adapter/UsbAdapter.cpp
index 64802b9..fb6ce11 100644
--- a/src/usb-adapter/UsbAdapter.cpp
+++ b/src/usb-adapter/UsbAdapter.cpp
@@ -43,8 +43,8 @@ bool GetChipInfo(const std::string chip, struct gpiochip_info &info) {
 }
 
 std::string GetCh341ChipFilename() {
-  static const char *ch341ChipLabel = "ch341";
-  static const size_t ch341NbLines = 16;
+  static const char *ch341ChipLabel = "ch341-gpio.1.auto";
+  static const size_t ch341NbLines = 18;
 
   auto chips = GetChipChipNames();
   for (auto &chip : chips) {
@@ -74,6 +74,10 @@ int GetGpioLineIndex(const std::string &chip, const char *name) {
 
     if (strcmp(name, infoLine.name) == 0)
       return i;
+    if (strcmp(name, "ini") == 0)
+      return 16;
+    if (strcmp(name, "slct") == 0)
+      return 11;
   }
   throw std::runtime_error("Cannot find GPIO line");
 }

With those changes bidirectional communication seems to work as well as it did with the recommended ch341 driver.

@mr-nice
Copy link
Contributor

mr-nice commented Nov 12, 2023

Cool @Djhg2000 , thanks for testing this out :)
I will prepare a PR to add the rest of the GPIOs. But for that I need to claim the GPIOs from the i2c driver and for that I need to read some documentation first.
I also need to do some testing for the bit 12 GPIO as I think it is not save to export it as output. And don't make sense to use it as input either. I think it is the internal reset for the cha341 itself.

@Djhg2000
Copy link

Djhg2000 commented Nov 12, 2023

One potential improvement is to supply names for the lines to the GPIO subsystem. The demo code I hacked on yesterday was written to look up the proper lines through the names, so the second part of my patch works around that by essentially short-circuiting the logic and supplying the line indexes manually. It's a dirty hack but it worked for a quick test.

I realize this is outside the scope of the issue, but in case line 12 is an internal reset and you want to skip it then I would like the other ones to be named so I (and others) have a better chance of working out which line corresponds to which physical pin (currently, lines assigned to SPI are labelled but not named).

Named GPIO lines are nice to have anyway as a general feature of this driver but I can create a separate issue for that if @frank-zago would prefer.

@frank-zago
Copy link
Owner

A previous version of the drivers has GPIO names. However that creates a conflict and a kernel warning when 2 or more devices are plugged in, so I removed it. See bottom of https://lkml.org/lkml/2021/5/10/116

@Djhg2000
Copy link

I see. Maybe we could find some temporary workaround to this though, for instance I would be fine with having to write something to the sysfs interface for exposing names on a specific gpiochip.

On the other hand it seemed to be a bug and there's no mention of what exactly the warning was, maybe we should check again and see if we still get the warning? Unfortunately I only have one CH341A based device (or at least I can't remember having another one around somewhere) so I can't do it myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants