ssd1306: Support connecting SPI and I2C simultaneously #56887

soburi · 2023-04-16T04:18:52Z

Support connecting different display for each SPI and I2C at the same time.

In a case like DTS below.

&spi1 {
        ssd1306_spi: ssd1306@0 {
		compatible = "solomon,ssd1306fb";
                ...
        };

};

&i2c0 {
        ssd1306_i2c: ssd1306@3c {
		compatible = "solomon,ssd1306fb";
		...
	};
};

In addition, I'm doing the necessary correspondence for connecting multiple devices.

jfischer-no · 2023-04-19T13:30:09Z

drivers/display/ssd1306.c

-#if DT_INST_PROP(0, segment_remap) == 1
-#define SSD1306_PANEL_SEGMENT_REMAP	true
-#else
-#define SSD1306_PANEL_SEGMENT_REMAP	false
-#endif
-
-#if DT_INST_PROP(0, com_invdir) == 1
-#define SSD1306_PANEL_COM_INVDIR	true
-#else
-#define SSD1306_PANEL_COM_INVDIR	false
-#endif
-
-#if DT_INST_PROP(0, com_sequential) == 1
-#define SSD1306_COM_PINS_HW_CONFIG	SSD1306_SET_PADS_HW_SEQUENTIAL
-#else
-#define SSD1306_COM_PINS_HW_CONFIG	SSD1306_SET_PADS_HW_ALTERNATIVE
-#endif
-
-#define SSD1306_PANEL_NUMOF_PAGES	(DT_INST_PROP(0, height) / 8)


This patch should be first change to add multi instance support for this driver.

I reordered commits.

@soburi this is still the last patch, can you check again?

@fabiobaltieri
Thank you for pointing out. It's certainly not good.
I fixed it.

jfischer-no · 2023-04-19T13:37:30Z

drivers/display/ssd1306.c

-	struct spi_buf_set tx_bufs = {
-		.buffers = &tx_buf,
-		.count = 1
-	};


Please stop unnecessary code reformatting.

Restored it.
And also fix the follows.
https://github.com/zephyrproject-rtos/zephyr/compare/7b2136aabb677b5eec3e34fbaaacd305c2b29c89..48bb5d6f44bf05860258f87acc8b74676742d353#diff-7da6d958478082c2b7f2549d889797958fbf4226bbee0fadbf40e3907479afadL86-R82

jfischer-no · 2023-04-19T13:39:24Z

drivers/display/Kconfig.ssd1306

-config SSD1306_REVERSE_MODE
-	bool "SSD1306 reverse mode"
-	help
-	  SSD1306 reverse video mode.
-


This commit should be second change before multi instance support commit.

I reordered commits.

jfischer-no · 2023-04-19T13:43:25Z

drivers/display/ssd1306.c

+union ssd1306_bus {
+#if DT_ANY_INST_ON_BUS_STATUS_OKAY(i2c)
+	struct i2c_dt_spec i2c;
+#endif
+#if DT_ANY_INST_ON_BUS_STATUS_OKAY(spi)
+	struct spi_dt_spec spi;
+#endif
+};


Please just

union ssd1306_bus { struct i2c_dt_spec i2c; struct spi_dt_spec spi; };

and remove all this ifdefery.

Removed #if in SPI and I2C conditionals.
However, the following parts are left because they cause unreferenced warnings.

https://github.com/zephyrproject-rtos/zephyr/pull/56887/files#diff-7da6d958478082c2b7f2549d889797958fbf4226bbee0fadbf40e3907479afadR68
https://github.com/zephyrproject-rtos/zephyr/pull/56887/files#diff-7da6d958478082c2b7f2549d889797958fbf4226bbee0fadbf40e3907479afadR94

jfischer-no · 2023-04-19T13:44:41Z

drivers/display/ssd1306.c

+#define SSD1306_CONFIG_SPI(inst)                                                                   \
+	{                                                                                          \
+		.bus = {.spi = SPI_DT_SPEC_INST_GET(                                               \
+				inst, SPI_OP_MODE_MASTER | SPI_TRANSFER_MSB | SPI_WORD_SET(8),     \
+				0)},                                                               \
+		.reset = GPIO_DT_SPEC_INST_GET_OR(inst, reset_gpios, {0}),                         \
+		.bus_ready = ssd1306_bus_ready_spi, .write_bus = ssd1306_write_bus_spi,            \
+		.bus_name = ssd1306_bus_name_spi,                                                  \
+		IF_ENABLED(DT_ANY_INST_ON_BUS_STATUS_OKAY(spi),                                    \
+			   (.data_cmd = GPIO_DT_SPEC_INST_GET(inst, data_cmd_gpios), ))            \
+	}
+
+#define SSD1306_CONFIG_I2C(inst)                                                                   \
+	{                                                                                          \
+		.bus = {.i2c = I2C_DT_SPEC_INST_GET(inst)},                                        \
+		.reset = GPIO_DT_SPEC_INST_GET_OR(inst, reset_gpios, {0}),                         \
+		.bus_ready = ssd1306_bus_ready_i2c, .write_bus = ssd1306_write_bus_i2c,            \
+		.bus_name = ssd1306_bus_name_i2c,                                                  \
+		IF_ENABLED(DT_ANY_INST_ON_BUS_STATUS_OKAY(spi), (.data_cmd = {0}, ))               \
+	}
+
+#define SSD1306_DEFINE(inst)                                                                       \
+	static struct ssd1306_data ssd1306_driver_##inst;                                          \
+	static const struct ssd1306_config ssd1306_config_##inst =                                 \
+		COND_CODE_1(DT_INST_ON_BUS(inst, spi), (SSD1306_CONFIG_SPI(inst)),                 \
+			    (SSD1306_CONFIG_I2C(inst)));                                           \
+                                                                                                   \
+	DEVICE_DT_INST_DEFINE(inst, ssd1306_init, NULL, &ssd1306_driver_##inst,                    \
+			      &ssd1306_config_##inst, POST_KERNEL, CONFIG_DISPLAY_INIT_PRIORITY,   \
+			      &ssd1306_driver_api);
+
+DT_INST_FOREACH_STATUS_OKAY(SSD1306_DEFINE)


Multi instance support should be the last commit in this PR.

I reordered commits.

github-actions · 2023-06-19T00:29:51Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

fabiobaltieri · 2023-06-20T11:27:21Z

@erwango @jfischer-no could you do another review pass once you have a chance?

drivers/display/ssd1306.c

dts/bindings/display/solomon,ssd1306fb-common.yaml

boards/shields/ssd1306/Kconfig.defconfig

drivers/display/ssd1306.c

soburi · 2023-08-21T12:22:51Z

@erwango @mbolivar-ampere
Could you take a look please?

fabiobaltieri · 2023-08-29T18:08:45Z

@erwango @mbolivar-ampere ping

erwango

Ok for shield part.

jfischer-no · 2023-08-30T12:05:16Z

What changed again?

soburi · 2023-08-31T04:35:11Z

@jfischer-no

What changed again?

Sinse Aug 9,
I fixed about pointed by @mbolivar-ampere #56887 (review)

and I change to remove #undef DT_DRV_COMPAT line.

https://github.com/zephyrproject-rtos/zephyr/compare/382db6dd259f314d9527b2e25eedd2aa303a0c37..e4646cf81d7346a04b62e7497c8db786b5533297

soburi · 2023-09-05T07:14:49Z

@mbolivar-ampere
Could you take a look again, please?

Rename `DT_COMPAT_ON_BUS_INTERNAL` to `DT_HAS_COMPAT_ON_BUS_STATUS_OKAY` to make it a public DT API. It is helpful for code that handles multiple DT_DRV_COMPAT in one file, such as in the following cases. ``` #if (DT_HAS_COMPAT_ON_BUS_STATUS_OKAY(some_sensor, i2c) || \ DT_HAS_COMPAT_ON_BUS_STATUS_OKAY(another_sensor, i2c) ... #endif #define DT_DRV_COMPAT some_sensor DT_INST_FOREACH_STATUS_OKAY(DEFINE_SOME_SENSOR) #undef DT_DRV_COMPAT #define DT_DRV_COMPAT another_sensor DT_INST_FOREACH_STATUS_OKAY(DEFINE_ANOTHER_SENSOR) ``` Signed-off-by: TOKITA Hiroshi <[email protected]>

Store properties defined in dts in ssd1306_config's fields. And replace code that uses DT_INST_PROP (0, ...) by config properties. Signed-off-by: TOKITA Hiroshi <[email protected]>

When multiple devices are connected, the SSD1306_REVERSE_MODE setting cannot switch for each device. Add an equivalent setting to the devicetree properties to replace it. Signed-off-by: TOKITA Hiroshi <[email protected]>

Determine sh1106 from the `compatibility` value instead of the SSD1306_CONTROLLER_TYPE setting. Change the settings in `boards/shields/ssd1306/sh1106_128x64.overlay` to follow this change. Remove the SSD1306_CONTROLLER_TYPE from its Kconfig.defconfig, and set the `compatibility` to `sinowealth,sh1106`. Signed-off-by: TOKITA Hiroshi <[email protected]>

Support connecting different display for each SPI and I2C at the same time. In a case like DTS below. ``` &spi1 { ssd1306_spi: ssd1306@0 { compatible = "solomon,ssd1306fb"; ... }; }; &i2c0 { ssd1306_i2c: ssd1306@3c { compatible = "solomon,ssd1306fb"; ... }; }; ``` Signed-off-by: TOKITA Hiroshi <[email protected]>

soburi · 2023-09-08T22:32:50Z

@mbolivar-ampere
ping

zephyrbot added the area: Display label Apr 16, 2023

zephyrbot requested a review from jfischer-no April 16, 2023 04:19

soburi force-pushed the ssd1306_i2c_and_spi branch from f21ddbc to 37aae4b Compare April 16, 2023 04:20

soburi marked this pull request as draft April 16, 2023 05:07

soburi force-pushed the ssd1306_i2c_and_spi branch from 80eba62 to 5a9155f Compare April 16, 2023 08:11

soburi marked this pull request as ready for review April 16, 2023 08:12

soburi requested review from galak and erwango as code owners April 16, 2023 08:12

zephyrbot added area: Shields Shields (add-on boards) area: Devicetree Binding PR modifies or adds a Device Tree binding labels Apr 16, 2023

zephyrbot requested a review from avisconti April 16, 2023 08:13

zephyrbot assigned erwango Apr 16, 2023

soburi changed the title ~~drivers: display: ssd1306: Support connecting SPI and I2C at the same~~ ssd1306: Support connecting SPI and I2C simultaneously Apr 16, 2023

soburi force-pushed the ssd1306_i2c_and_spi branch from 5a9155f to 4e68896 Compare April 16, 2023 08:45

soburi marked this pull request as draft April 17, 2023 10:05

soburi force-pushed the ssd1306_i2c_and_spi branch from 4e68896 to 7b2136a Compare April 17, 2023 12:26

soburi marked this pull request as ready for review April 18, 2023 02:01

soburi marked this pull request as draft April 18, 2023 02:02

soburi marked this pull request as ready for review April 18, 2023 02:04

jfischer-no requested changes Apr 19, 2023

View reviewed changes

soburi force-pushed the ssd1306_i2c_and_spi branch 3 times, most recently from 7bea780 to 1a0dddb Compare April 19, 2023 21:37

soburi requested a review from jfischer-no April 19, 2023 22:07

github-actions bot added the Stale label Jun 19, 2023

github-actions bot removed the Stale label Jun 21, 2023

soburi force-pushed the ssd1306_i2c_and_spi branch from 1a0dddb to 0645aa1 Compare June 22, 2023 22:12

fabiobaltieri reviewed Jun 26, 2023

View reviewed changes

drivers/display/ssd1306.c Outdated Show resolved Hide resolved

dts/bindings/display/solomon,ssd1306fb-common.yaml Outdated Show resolved Hide resolved

boards/shields/ssd1306/Kconfig.defconfig Show resolved Hide resolved

drivers/display/ssd1306.c Outdated Show resolved Hide resolved

erwango previously approved these changes Aug 30, 2023

View reviewed changes

soburi dismissed stale reviews from erwango and fabiobaltieri via b3ebfcb August 31, 2023 04:26

soburi force-pushed the ssd1306_i2c_and_spi branch from 5b39a88 to b3ebfcb Compare August 31, 2023 04:26

zephyrbot requested a review from kartben August 31, 2023 04:26

jfischer-no previously approved these changes Aug 31, 2023

View reviewed changes

soburi requested review from fabiobaltieri and erwango August 31, 2023 07:17

fabiobaltieri previously approved these changes Aug 31, 2023

View reviewed changes

erwango previously approved these changes Aug 31, 2023

View reviewed changes

soburi added 5 commits September 9, 2023 06:50

drivers: display: ssd1306: add dts properties to ssd1306_config

e2f4740

Store properties defined in dts in ssd1306_config's fields. And replace code that uses DT_INST_PROP (0, ...) by config properties. Signed-off-by: TOKITA Hiroshi <[email protected]>

soburi dismissed stale reviews from erwango, fabiobaltieri, and jfischer-no via e6ece74 September 8, 2023 22:16

soburi force-pushed the ssd1306_i2c_and_spi branch from b3ebfcb to e6ece74 Compare September 8, 2023 22:16

fabiobaltieri approved these changes Sep 8, 2023

View reviewed changes

xyzzy42 mentioned this pull request Sep 9, 2023

Fixes and additional sensor support for Si7006, SHT21, HTU21D sensors #56227

Merged

mbolivar-ampere approved these changes Sep 13, 2023

View reviewed changes

mbolivar-ampere merged commit 9fcfb31 into zephyrproject-rtos:main Sep 13, 2023

soburi deleted the ssd1306_i2c_and_spi branch September 13, 2023 21:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ssd1306: Support connecting SPI and I2C simultaneously #56887

ssd1306: Support connecting SPI and I2C simultaneously #56887

soburi commented Apr 16, 2023 •

edited

Loading

jfischer-no Apr 19, 2023

soburi Apr 19, 2023

fabiobaltieri Jun 20, 2023

soburi Jun 22, 2023

jfischer-no Apr 19, 2023

soburi Apr 19, 2023

jfischer-no Apr 19, 2023

soburi Apr 19, 2023

jfischer-no Apr 19, 2023

soburi Apr 19, 2023

jfischer-no Apr 19, 2023

soburi Apr 19, 2023

github-actions bot commented Jun 19, 2023

fabiobaltieri commented Jun 20, 2023

soburi commented Aug 21, 2023 •

edited

Loading

fabiobaltieri commented Aug 29, 2023

erwango left a comment

jfischer-no commented Aug 30, 2023

soburi commented Aug 31, 2023

soburi commented Sep 5, 2023

soburi commented Sep 8, 2023

ssd1306: Support connecting SPI and I2C simultaneously #56887

ssd1306: Support connecting SPI and I2C simultaneously #56887

Conversation

soburi commented Apr 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 19, 2023

fabiobaltieri commented Jun 20, 2023

soburi commented Aug 21, 2023 • edited Loading

fabiobaltieri commented Aug 29, 2023

erwango left a comment

Choose a reason for hiding this comment

jfischer-no commented Aug 30, 2023

soburi commented Aug 31, 2023

soburi commented Sep 5, 2023

soburi commented Sep 8, 2023

soburi commented Apr 16, 2023 •

edited

Loading

soburi commented Aug 21, 2023 •

edited

Loading