* Re: [PATCH v2 3/4] drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support
From: Amit Barzilai @ 2026-06-23 21:34 UTC (permalink / raw)
To: markus.elfring
Cc: airlied, amit.barzilai22, andy, azuddinadam, chintanlike,
conor+dt, deller, devicetree, dri-devel, gregkh, javierm, krzk+dt,
linux-fbdev, linux-kernel, linux-staging, maarten.lankhorst,
mripard, robh, simona, tzimmermann
In-Reply-To: <16a86f3d-caf0-46d0-97a4-c9585bdaa06c@web.de>
Markus Elfring <Markus.Elfring@web.de> writes:
>> +++ b/drivers/gpu/drm/solomon/ssd130x.c
>> @@ -146,6 +146,33 @@
>> #define SSD133X_COLOR_DEPTH_256 0x0
>> #define SSD133X_COLOR_DEPTH_65K 0x1
>>
>> +/* ssd135x commands */
>> +#define SSD135X_SET_COL_RANGE 0x15
>> +#define SSD135X_WRITE_RAM 0x5c
>> +#define SSD135X_SET_ROW_RANGE 0x75
> [...]
>
> How do you think about to use an enumeration for such data?
> https://en.wikipedia.org/wiki/Enumerated_type#C_and_syntactically_similar_languages
Thank you for the suggestion.
I used #define to stay consistent with the rest of ssd130x.c, where the
command constants for the other families are all defined the same way.
In my opinion an enum could be a readable solution for these values, but I
don't think the switch should be included in this series.
--
Thanks,
Amit
^ permalink raw reply
* Re: [PATCH v2 4/4] staging: fbtft: remove fb_ssd1351 driver
From: Amit Barzilai @ 2026-06-23 20:28 UTC (permalink / raw)
To: javierm, andriy.shevchenko, mripard
Cc: airlied, andy, azuddinadam, chintanlike, conor+dt, deller,
devicetree, dri-devel, gregkh, krzk+dt, linux-fbdev, linux-kernel,
linux-staging, maarten.lankhorst, robh, simona, tzimmermann
In-Reply-To: <87ik79iqsz.fsf@ocarina.mail-host-address-is-not-set>
Javier Martinez Canillas <javierm@redhat.com> writes:
> Andy Shevchenko <andriy.shevchenko@intel.com> writes:
>
>> On Mon, Jun 22, 2026 at 06:25:06PM +0300, Amit Barzilai wrote:
>>> The SSD1351 support was added to the ssd130x DRM driver. To avoid
>>> confusion and irrelevant updates, the staging fb_ssd1351 driver is
>>> removed.
>>
>> NAK, the fbtft has two drivers in one (SPI + parallel), plus as Maxime said,
>> it has its own binding.
>>
>
> Yes, I agree that we don't want to delete the fbtft driver.
I will remove this patch from the next version.
Thank you Javier, Andy and Maxime for the feedback and explanations.
--
Thanks,
Amit
^ permalink raw reply
* Re: [PATCH 3/3] backlight: lp8864: Convert from LED to backlight class driver
From: Daniel Thompson @ 2026-06-23 13:17 UTC (permalink / raw)
To: Sverdlin, Alexander
Cc: dri-devel@lists.freedesktop.org, danielt@kernel.org,
linux-leds@vger.kernel.org, devicetree@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <aa34231591d2e5f848bdd74524abb4fe85123444.camel@siemens.com>
On Tue, Jun 23, 2026 at 11:59:16AM +0000, Sverdlin, Alexander wrote:
> Hi Daniel,
>
> On Tue, 2026-06-23 at 12:41 +0100, Daniel Thompson wrote:
> > > The mechanism we have regarding hot plugging currently is just I2C bridge,
> > > which de-registers and registers the bridged bus. So no additional drivers
> > > are required, as long as I2C devices are self-contained and not glued with
> > > platform devices.
> > >
> > > So bottom line is, I'd prefer to just add the backlight interface to the
> > > existing driver, no matter where it would live in the future.
> >
> > Is there any reasion that LP8864/LP8866 is unique in appearing on a
> > hotplugged I2C bus? In other words if support for dynamism is added
> > specifically to leds-lp8864.c rather than in led_bl.c then what will stop
> > the same dynamic tricks from being adde to other LED drivers?
>
> LP8864 is not special in this regard, not different from adp8860_bl.c,
> adp8870_bl.c or lm3639_bl.c, which would work in my application.
The most recent of these drivers adopted LED support in 2012 whilst
led_bl.c was introduced in 2020.
> So I just wanted to add currently missing backlight_device_register() to
> LP8864, but there were no such dual-API drivers under drivers/leds/ that's
> wht I started with moving LP8864 driver into drivers/video/backlight/.
>
> led_bl.c however could only be hot-pluggable either if one actively
> creates/destroys platform device somewhere in the code programmatically
> or uses DT overlays.
>
> Just adding backlight API to LP8864 would be much more idiomatic, I'd
> say.
Maybe.
I think I might consider the drivers above to be historical anomalies
rather than idiomatic examples.
Daniel.
^ permalink raw reply
* Re: [PATCH 3/3] backlight: lp8864: Convert from LED to backlight class driver
From: Sverdlin, Alexander @ 2026-06-23 11:59 UTC (permalink / raw)
To: daniel@riscstar.com
Cc: dri-devel@lists.freedesktop.org, danielt@kernel.org,
linux-leds@vger.kernel.org, devicetree@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <ajpw1w150SXm8Gbi@aspen.lan>
Hi Daniel,
On Tue, 2026-06-23 at 12:41 +0100, Daniel Thompson wrote:
> > The mechanism we have regarding hot plugging currently is just I2C bridge,
> > which de-registers and registers the bridged bus. So no additional drivers
> > are required, as long as I2C devices are self-contained and not glued with
> > platform devices.
> >
> > So bottom line is, I'd prefer to just add the backlight interface to the
> > existing driver, no matter where it would live in the future.
>
> Is there any reasion that LP8864/LP8866 is unique in appearing on a
> hotplugged I2C bus? In other words if support for dynamism is added
> specifically to leds-lp8864.c rather than in led_bl.c then what will stop
> the same dynamic tricks from being adde to other LED drivers?
LP8864 is not special in this regard, not different from adp8860_bl.c,
adp8870_bl.c or lm3639_bl.c, which would work in my application.
So I just wanted to add currently missing backlight_device_register() to
LP8864, but there were no such dual-API drivers under drivers/leds/ that's
wht I started with moving LP8864 driver into drivers/video/backlight/.
led_bl.c however could only be hot-pluggable either if one actively
creates/destroys platform device somewhere in the code programmatically
or uses DT overlays.
Just adding backlight API to LP8864 would be much more idiomatic, I'd
say.
--
Alexander Sverdlin
Siemens AG
www.siemens.com
^ permalink raw reply
* Re: [PATCH 3/3] backlight: lp8864: Convert from LED to backlight class driver
From: Daniel Thompson @ 2026-06-23 11:41 UTC (permalink / raw)
To: Sverdlin, Alexander
Cc: linux-leds@vger.kernel.org, afd@ti.com, jingoohan1@gmail.com,
linux-fbdev@vger.kernel.org, pavel@kernel.org, lee@kernel.org,
dri-devel@lists.freedesktop.org, robh@kernel.org,
linux-kernel@vger.kernel.org, krzk+dt@kernel.org,
danielt@kernel.org, conor+dt@kernel.org, deller@gmx.de,
devicetree@vger.kernel.org
In-Reply-To: <acb13aca040ab72b9e53abee3a5c8733ebc5b8dd.camel@siemens.com>
On Tue, Jun 16, 2026 at 07:17:23AM +0000, Sverdlin, Alexander wrote:
> Hi Andrew!
>
> On Mon, 2026-06-15 at 14:51 -0500, Andrew Davis wrote:
> > > Move the TI LP8864/LP8866 driver from drivers/leds/ to
> > > drivers/video/backlight/
> >
> > Why move it? You can register a backlight device from any directory.
>
> I'm personally fine with the driver residing in drivers/leds, it's
> just that currently there are no combined drivers there, the combined
> drivers providing both interfaces only live in video/backlight.
>
> But if it's OK from the maintainers' perspective, it will be even
> more consistent regarding Kconfig symbol.
>
> > > and convert it to register a backlight class
> > > device as its primary interface.
> > >
> >
> > What do you mean by "primary"? You should be able to register with
> > both frameworks and have the driver interop between as needed.
>
> Well, I only meant the user's (or my own) perspective, sorry for confusion.
>
> > > The motivation is a use case on a hot-pluggable segment of an I2C bus.
> > > The generic led-backlight driver (drivers/video/backlight/led_bl.c) is a
> > > platform driver and as such inherently non-hotpluggable.
> >
> > That isn't strictly true, there is platform_device_{del,unregister}(), so
> > whatever your mechanism for removing the I2C device would be, the same
> > could be done to the led_bl device before then removing the I2C device.
>
> led_bl is not really designed to act on dynamically instantiated devices,
> it's very much device-tree affine (of_count_phandle_with_args(), etc...)
>
> > We don't want to have to move every LED driver that could possibly
> > be used as a backlight to the backlight framework, the led_bl.c
> > handles adapting LED->backlight as needed. So what you really need
> > here is to de-couple led_bl.c from DT so it can better handle dynamic
> > add/remove. Then this LED driver simply could register a "led-backlight"
> > platform driver to handle the backlight interface, and remove the
> > backlight device when it itself (the LED device) is removed.
>
> The mechanism we have regarding hot plugging currently is just I2C bridge,
> which de-registers and registers the bridged bus. So no additional drivers
> are required, as long as I2C devices are self-contained and not glued with
> platform devices.
>
> So bottom line is, I'd prefer to just add the backlight interface to the
> existing driver, no matter where it would live in the future.
Is there any reasion that LP8864/LP8866 is unique in appearing on a
hotplugged I2C bus? In other words if support for dynamism is added
specifically to leds-lp8864.c rather than in led_bl.c then what will stop
the same dynamic tricks from being adde to other LED drivers?
Daniel.
^ permalink raw reply
* Re: [PATCH v2 3/4] drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support
From: Andy Shevchenko @ 2026-06-23 9:37 UTC (permalink / raw)
To: Amit Barzilai
Cc: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller, azuddinadam,
chintanlike, dri-devel, devicetree, linux-kernel, linux-fbdev,
linux-staging
In-Reply-To: <20260622152506.78627-4-amit.barzilai22@gmail.com>
On Mon, Jun 22, 2026 at 06:25:05PM +0300, Amit Barzilai wrote:
> The Solomon SSD1351 is a 128x128 RGB color OLED controller. It shares
> the SSD133X data path: a column/row addressing window followed by a bulk
> RGB565 pixel write. Add it as a new SSD135X_FAMILY rather than a separate
> driver, reusing the SSD133X plane, CRTC and blit/clear helpers.
>
> The only data-path difference is that the SSD1351 requires an explicit
> Write RAM command (0x5c) after the address window is programmed, before
> pixel data is accepted, whereas the SSD133X enters data mode implicitly.
> This is emitted from a shared ssd133x_write_pixels() helper so both the
> damage-update and clear-screen paths cover it.
>
> The SSD1351 also needs its own init sequence (ssd135x_init), dispatched
> via ssd135x_encoder_atomic_enable, and a longer post-reset settle delay.
> The re-map byte is fixed at 0 degrees, 65k color, COM split, BGR
> sub-pixel order; rotation is not supported.
>
> The SSD1351 is SPI-only, so only the SPI transport match tables gain an
> entry; no new config symbol is needed.
...
> const struct ssd130x_deviceinfo ssd130x_variants[] = {
> .default_height = 64,
> .format_rgb565 = 1,
> .family_id = SSD133X_FAMILY,
> + },
> + /* ssd135x family */
> + [SSD1351_ID] = {
> + .default_width = 128,
> + .default_height = 128,
> + .format_rgb565 = 1,
> + .family_id = SSD135X_FAMILY,
> }
While it's not a problem _in this case_, the rule of thumb is always to have a
trailing comma for non-terminator entry.
...
> /*
> * Helper to write data (SSD13XX_DATA) to the device.
> */
> -static int ssd130x_write_data(struct ssd130x_device *ssd130x, u8 *values, int count)
> +static int ssd130x_write_data(struct ssd130x_device *ssd130x, const u8 *values, int count)
> {
> return regmap_bulk_write(ssd130x->regmap, SSD13XX_DATA, values, count);
> }
Stray change. If needed, either explain in the commit message or create
a separate patch (depending on the dependencies).
...
> unsigned int i;
> int ret;
>
> + /*
> + * The SSD135X family latches command parameters with D/C# HIGH (i.e.
> + * clocked in as data), unlike the other families where the opcode and
> + * all of its parameters are sent as commands (D/C# LOW). Send the
> + * opcode as a command and any following parameter bytes as data.
> + */
> + if (ssd130x->device_info->family_id == SSD135X_FAMILY) {
> + if (len == 0)
> + return 0;
> + ret = regmap_write(ssd130x->regmap, SSD13XX_COMMAND, cmd[0]);
> + if (ret || len == 1)
> + return ret;
> +
> + return ssd130x_write_data(ssd130x, cmd + 1, len - 1);
> + }
> for (i = 0; i < len; i++) {
This loop seems for the len, so it will be the same for both devices as far as
I can see the context. I can't find this piece in the original driver, perhaps
it's some dependency?
> ret = regmap_write(ssd130x->regmap, SSD13XX_COMMAND, cmd[i]);
> if (ret)
...
> +/*
> + * Variadic wrapper around ssd130x_write_cmds(). The first variadic argument is
> + * the command opcode and the following ones are its options/parameters.
> + */
> +static int ssd130x_write_cmd(struct ssd130x_device *ssd130x, int count,
> + /* u8 cmd, u8 option, ... */...)
> +{
> + u8 buf[8];
> + va_list ap;
> + int i;
> +
> + if (count > ARRAY_SIZE(buf))
> + return -EINVAL;
> +
> + va_start(ap, count);
> + for (i = 0; i < count; i++)
Can be
for (int i = 0; i < count; i++)
> + buf[i] = va_arg(ap, int);
> + va_end(ap);
> +
> + return ssd130x_write_cmds(ssd130x, buf, count);
> +}
...
> +static int ssd135x_init(struct ssd130x_device *ssd130x)
> +{
> + /*
> + * Horizontal address increment, COM split, reversed COM scan direction,
> + * BGR sub-pixel order and 65k (RGB565) color depth. Rotation is not
> + * supported, so the remap byte is fixed.
> + */
> + u8 remap = SSD135X_SET_REMAP_65K | SSD135X_SET_REMAP_COM_SPLIT |
> + SSD135X_SET_REMAP_COLOR_BGR | SSD135X_SET_REMAP_COM_SCAN;
> + const u8 cmds[] = {
Why not static?
> + 2, SSD135X_SET_COMMAND_LOCK, 0x12,
> + 2, SSD135X_SET_COMMAND_LOCK, 0xb1,
> + 1, SSD13XX_DISPLAY_OFF,
> + 2, SSD135X_SET_CLOCK_FREQ, 0xf1,
> + 2, SSD135X_SET_MUX_RATIO, ssd130x->height - 1,
> + 3, SSD135X_SET_COL_RANGE, 0x00, ssd130x->width - 1,
> + 3, SSD135X_SET_ROW_RANGE, 0x00, ssd130x->height - 1,
> + 2, SSD135X_SET_DISPLAY_START, 0x00,
> + 2, SSD135X_SET_DISPLAY_OFFSET, 0x00,
> + 2, SSD135X_SET_GPIO, 0x00,
> + 2, SSD135X_SET_FUNCTION, 0x01,
> + 2, SSD135X_SET_PHASE_LENGTH, 0x32,
> + 4, SSD135X_SET_VSL, 0xa0, 0xb5, 0x55,
> + 2, SSD135X_SET_PRECHARGE, 0x17,
> + 2, SSD135X_SET_VCOMH_VOLTAGE, 0x05,
> + 4, SSD135X_SET_CONTRAST, 0xc8, 0x80, 0xc8,
> + 2, SSD135X_SET_CONTRAST_MASTER, 0x0f,
> + 2, SSD135X_SET_PRECHARGE2, 0x01,
> + 1, SSD135X_SET_DISPLAY_NORMAL,
> + 2, SSD13XX_SET_SEG_REMAP, remap,
> + 0,
No trailing comma for the terminator entry.
> + };
> +
> + /*
> + * ssd130x_power_on() issues a short reset pulse, but the SSD1351 is not
> + * ready to accept commands immediately afterwards. Give the controller
> + * time to settle before sending the init sequence.
> + */
Any reference to the datasheet?
> + msleep(120);
> +
> + return ssd130x_run_cmd_seq(ssd130x, cmds);
> +}
...
> +/*
> + * Write a run of pixel data to the controller's display RAM. The SSD135X
> + * family requires an explicit Write RAM command once the address window has
> + * been set, before any pixel data is accepted; the SSD133X family enters data
> + * mode implicitly after the column/row range is programmed.
> + */
> +static int ssd133x_write_pixels(struct ssd130x_device *ssd130x,
> + u8 *data_array, unsigned int count)
> +{
> + if (ssd130x->device_info->family_id == SSD135X_FAMILY) {
> + int ret = ssd130x_write_cmd(ssd130x, 1, SSD135X_WRITE_RAM);
> +
> + if (ret < 0)
> + return ret;
This style is discouraged as it's harder to maintain. Better to split
assignment and definition
int ret;
ret = ssd130x_write_cmd(ssd130x, 1, SSD135X_WRITE_RAM);
if (ret < 0)
return ret;
> + }
> +
> + return ssd130x_write_data(ssd130x, data_array, count);
> +}
...
> static const struct drm_plane_helper_funcs ssd130x_primary_plane_helper_funcs[]
> .atomic_check = ssd133x_primary_plane_atomic_check,
> .atomic_update = ssd133x_primary_plane_atomic_update,
> .atomic_disable = ssd133x_primary_plane_atomic_disable,
> + },
> + [SSD135X_FAMILY] = {
> + DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
> + .atomic_check = ssd133x_primary_plane_atomic_check,
> + .atomic_update = ssd133x_primary_plane_atomic_update,
> + .atomic_disable = ssd133x_primary_plane_atomic_disable,
> }
As per another similar case.
> };
...
> static const struct drm_encoder_helper_funcs ssd130x_encoder_helper_funcs[] = {
> [SSD133X_FAMILY] = {
> .atomic_enable = ssd133x_encoder_atomic_enable,
> .atomic_disable = ssd130x_encoder_atomic_disable,
> + },
> + [SSD135X_FAMILY] = {
> + .atomic_enable = ssd135x_encoder_atomic_enable,
> + .atomic_disable = ssd130x_encoder_atomic_disable,
> }
> };
Ditto.
...
> diff --git a/drivers/gpu/drm/solomon/ssd130x.h b/drivers/gpu/drm/solomon/ssd130x.h
> index b0b487c06e04..da89d4455270 100644
> --- a/drivers/gpu/drm/solomon/ssd130x.h
> +++ b/drivers/gpu/drm/solomon/ssd130x.h
> @@ -26,7 +26,8 @@
> enum ssd130x_family_ids {
> SSD130X_FAMILY,
> SSD132X_FAMILY,
> - SSD133X_FAMILY
> + SSD133X_FAMILY,
> + SSD135X_FAMILY
Ditto, and this is exactly the whole point why non-terminator entries should
have a trailing comma.
> };
...
> enum ssd130x_variants {
> SSD1327_ID,
> /* ssd133x family */
> SSD1331_ID,
> + /* ssd135x family */
> + SSD1351_ID,
> NR_SSD130X_VARIANTS
See the difference? Here is terminator, which is clear. The above cases are
not.
> };
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 3/4] drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support
From: Markus Elfring @ 2026-06-23 9:05 UTC (permalink / raw)
To: Amit Barzilai, dri-devel, devicetree, linux-fbdev, linux-staging,
Andy Shevchenko, Conor Dooley, David Airlie, Greg Kroah-Hartman,
Helge Deller, Javier Martinez Canillas, Krzysztof Kozlowski,
Maarten Lankhorst, Maxime Ripard, Rob Herring, Simona Vetter,
Thomas Zimmermann
Cc: LKML, Adam Azuddin, Chintan Patel
In-Reply-To: <20260622152506.78627-4-amit.barzilai22@gmail.com>
…
> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> @@ -146,6 +146,33 @@
> #define SSD133X_COLOR_DEPTH_256 0x0
> #define SSD133X_COLOR_DEPTH_65K 0x1
>
> +/* ssd135x commands */
> +#define SSD135X_SET_COL_RANGE 0x15
> +#define SSD135X_WRITE_RAM 0x5c
> +#define SSD135X_SET_ROW_RANGE 0x75
…
How do you think about to use an enumeration for such data?
https://en.wikipedia.org/wiki/Enumerated_type#C_and_syntactically_similar_languages
Regards,
Markus
^ permalink raw reply
* Re: [PATCH v2 2/4] drm/ssd130x: Add RGB565 support to SSD133X family
From: Andy Shevchenko @ 2026-06-23 9:03 UTC (permalink / raw)
To: Amit Barzilai
Cc: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller, azuddinadam,
chintanlike, dri-devel, devicetree, linux-kernel, linux-fbdev,
linux-staging
In-Reply-To: <20260622152506.78627-3-amit.barzilai22@gmail.com>
On Mon, Jun 22, 2026 at 06:25:04PM +0300, Amit Barzilai wrote:
> SSD133X screens were getting 8bpp (RGB332) instead of the 16bpp
> (RGB565) that they support. This change adds a boolean to the
> deviceinfo struct selecting whether the variant is driven at
> DRM_FORMAT_RGB565.
>
> Changed SSD133X to now utilize 65k color (RGB565).
...
> - * Each Segment has a 8-bit pixel and each Common output has a
> - * row of pixels. When using the (default) horizontal address
> - * increment mode, each byte of data sent to the controller has
> - * a Segment (e.g: SEG0).
> + * Each Segment holds one pixel and each Common output has a row
> + * of pixels. A pixel is 8 bits (one byte) in the 256 color
> + * (RGB332) format or 16 bits (two bytes) in the 65k color
> + * (RGB565) format. When using the (default) horizontal address
> + * increment mode, the pixel data is sent Segment by Segment
> + * (e.g: SEG0 first).
> *
> * When using the 256 color depth format, each pixel contains 3
> * sub-pixels for color A, B and C. These have 3 bit, 3 bit and
> * 2 bits respectively.
Something wrong with the plural. There is a difference between "3-bit" and
"3 bits", but "3 bit" is odd.
> + *
> + * When using the 65k color depth format, each pixel contains 3
> + * sub-pixels for color A, B and C. These have 5 bit, 6 bit and
> + * 5 bits respectively.
Same mistake is repeated here.
...
> + /*
> + * Per-variant output format selector for the SSD133X data path. The
> + * hardware can drive the panel in RGB332 (1 byte/pixel) or RGB565
> + * (2 bytes/pixel); this is a policy choice per variant, not a
In other comments it was spelled fully, be consistent "1 byte per pixel",
"2 bytes per pixel".
> + * capability probe. When set, the variant is driven at RGB565.
> + */
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 4/4] staging: fbtft: remove fb_ssd1351 driver
From: Javier Martinez Canillas @ 2026-06-23 8:50 UTC (permalink / raw)
To: Andy Shevchenko, Amit Barzilai
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona, robh,
krzk+dt, conor+dt, andy, gregkh, deller, azuddinadam, chintanlike,
dri-devel, devicetree, linux-kernel, linux-fbdev, linux-staging
In-Reply-To: <ajpGy7K4eM8oJIfD@ashevche-desk.local>
Andy Shevchenko <andriy.shevchenko@intel.com> writes:
> On Mon, Jun 22, 2026 at 06:25:06PM +0300, Amit Barzilai wrote:
>> The SSD1351 support was added to the ssd130x DRM driver. To avoid
>> confusion and irrelevant updates, the staging fb_ssd1351 driver is
>> removed.
>
> NAK, the fbtft has two drivers in one (SPI + parallel), plus as Maxime said,
> it has its own binding.
>
Yes, I agree that we don't want to delete the fbtft driver.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: display: Add Solomon SSD1351 OLED controller
From: Krzysztof Kozlowski @ 2026-06-23 8:43 UTC (permalink / raw)
To: Amit Barzilai
Cc: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller, azuddinadam,
chintanlike, dri-devel, devicetree, linux-kernel, linux-fbdev,
linux-staging
In-Reply-To: <20260622152506.78627-2-amit.barzilai22@gmail.com>
On Mon, Jun 22, 2026 at 06:25:03PM +0300, Amit Barzilai wrote:
> Add a device tree binding for the Solomon SSD1351, a 128x128 65k-color
> RGB OLED display controller driven over a 4-wire SPI bus. The binding
> builds on the shared solomon,ssd-common.yaml properties already used by
> the other Solomon display controllers.
>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> .../bindings/display/solomon,ssd1351.yaml | 42 +++++++++++++++++++
> 1 file changed, 42 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd1351.yaml
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 4/4] staging: fbtft: remove fb_ssd1351 driver
From: Andy Shevchenko @ 2026-06-23 8:41 UTC (permalink / raw)
To: Amit Barzilai
Cc: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller, azuddinadam,
chintanlike, dri-devel, devicetree, linux-kernel, linux-fbdev,
linux-staging
In-Reply-To: <20260622152506.78627-5-amit.barzilai22@gmail.com>
On Mon, Jun 22, 2026 at 06:25:06PM +0300, Amit Barzilai wrote:
> The SSD1351 support was added to the ssd130x DRM driver. To avoid
> confusion and irrelevant updates, the staging fb_ssd1351 driver is
> removed.
NAK, the fbtft has two drivers in one (SPI + parallel), plus as Maxime said,
it has its own binding.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] video: fbdev: pm2fb: unwind WC setup on probe failure
From: Helge Deller @ 2026-06-23 8:19 UTC (permalink / raw)
To: Haoxiang Li; +Cc: linux-fbdev, linux-kernel
In-Reply-To: <20260621071935.3802673-1-haoxiang_li2024@163.com>
On 6/21/26 09:19, Haoxiang Li wrote:
> Add arch_phys_wc_del() on error path to keep the
> write-combining setup balanced when later probe
> steps fail.
>
> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
> ---
> drivers/video/fbdev/pm2fb.c | 1 +
> 1 file changed, 1 insertion(+)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH] video: hpfb: Unregister DIO driver on init failure
From: Helge Deller @ 2026-06-23 8:10 UTC (permalink / raw)
To: Haoxiang Li; +Cc: linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <20260622064915.767194-1-haoxiang_li2024@163.com>
On 6/22/26 08:49, Haoxiang Li wrote:
> hpfb_init() registers the DIO driver via dio_register_driver().
> If a later error occurs, the function returns directly without
> unregistering the DIO driver. Unregister the DIO driver before
> returning from these error paths.
>
> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
> ---
> drivers/video/fbdev/hpfb.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
Thanks! Patch applied to fbdev git tree.
By the way... doesn't the request_mem_region() below needs a corresponding
release_mem_region() in the failure path?
Helge
>
> diff --git a/drivers/video/fbdev/hpfb.c b/drivers/video/fbdev/hpfb.c
> index a1144b150982..1405712e5b4f 100644
> --- a/drivers/video/fbdev/hpfb.c
> +++ b/drivers/video/fbdev/hpfb.c
> @@ -407,10 +407,13 @@ static int __init hpfb_init(void)
> err = copy_from_kernel_nofault(&i, (unsigned char *)INTFBVADDR + DIO_IDOFF, 1);
>
> if (!err && (i == DIO_ID_FBUFFER) && topcat_sid_ok(sid = DIO_SECID(INTFBVADDR))) {
> - if (!request_mem_region(INTFBPADDR, DIO_DEVSIZE, "Internal Topcat"))
> + if (!request_mem_region(INTFBPADDR, DIO_DEVSIZE, "Internal Topcat")) {
> + dio_unregister_driver(&hpfb_driver);
> return -EBUSY;
> + }
> printk(KERN_INFO "Internal Topcat found (secondary id %02x)\n", sid);
> if (hpfb_init_one(INTFBPADDR, INTFBVADDR)) {
> + dio_unregister_driver(&hpfb_driver);
> return -ENOMEM;
> }
> }
^ permalink raw reply
* Re: [PATCH v3] fbdev: claim Cyber2000 SPARC I/O aperture before ioremap
From: Helge Deller @ 2026-06-23 7:57 UTC (permalink / raw)
To: Chintan Patel, linux; +Cc: tzimmermann, linux-fbdev, dri-devel
In-Reply-To: <20260616043035.11952-1-chintanlike@gmail.com>
On 6/16/26 06:30, Chintan Patel wrote:
> Claim the memory resource associated with the Cyber2000 SPARC MMIO
> aperture before accessing it.
>
> This is part of the effort to request memory regions in fbdev drivers.
IMHO this patch doesn't make much sense.
The PCI regions (e.g. for x86) are already requested one level higher and are being kept reserved.
This patch here only touches the SPARC code inside a path which is e.g. being used everytime the machine wakes up from sleep (does sparc even suspends?). So, instead here, I think (if you want to make this robust) the region should be reserved e.g. in cyberpro_common_probe(), but I'm not sure if this is even worth the effort....?
Helge
>
> Signed-off-by: Chintan Patel <chintanlike@gmail.com>
> ---
> v3:
> - Use pr_err() instead of pci_err()
> v2:
> - Use pci_err() for error reporting instead of printk().
>
> drivers/video/fbdev/cyber2000fb.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/cyber2000fb.c b/drivers/video/fbdev/cyber2000fb.c
> index 2d12f8e96c7e..9fced6bf6ffd 100644
> --- a/drivers/video/fbdev/cyber2000fb.c
> +++ b/drivers/video/fbdev/cyber2000fb.c
> @@ -47,6 +47,7 @@
> #include <linux/io.h>
> #include <linux/i2c.h>
> #include <linux/i2c-algo-bit.h>
> +#include <linux/ioport.h>
>
> #ifdef __arm__
> #include <asm/mach-types.h>
> @@ -1620,9 +1621,14 @@ static int cyberpro_pci_enable_mmio(struct cfb_info *cfb)
> */
> unsigned char __iomem *iop;
>
> + if (!request_mem_region(0x3000000, 0x5000, "cyber2000fb iop")) {
> + pr_err("iga5000: cannot reserve I/O area 0x3000000\n");
> + return -EBUSY;
> + }
> iop = ioremap(0x3000000, 0x5000);
> if (iop == NULL) {
> - printk(KERN_ERR "iga5000: cannot map I/O\n");
> + pr_err("iga5000: cannot map I/O area\n");
> + release_mem_region(0x3000000, 0x5000);
> return -ENOMEM;
> }
>
> @@ -1633,6 +1639,7 @@ static int cyberpro_pci_enable_mmio(struct cfb_info *cfb)
> writeb(EXT_BIU_MISC_LIN_ENABLE, iop + 0x3cf);
>
> iounmap(iop);
> + release_mem_region(0x3000000, 0x5000);
> #else
> /*
> * Most other machine types are "normal", so
^ permalink raw reply
* Re: [PATCH v2 4/4] staging: fbtft: remove fb_ssd1351 driver
From: Maxime Ripard @ 2026-06-23 7:55 UTC (permalink / raw)
To: Amit Barzilai
Cc: javierm, maarten.lankhorst, tzimmermann, airlied, simona, robh,
krzk+dt, conor+dt, andy, gregkh, deller, azuddinadam, chintanlike,
dri-devel, devicetree, linux-kernel, linux-fbdev, linux-staging
In-Reply-To: <20260622152506.78627-5-amit.barzilai22@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 404 bytes --]
Hi,
On Mon, Jun 22, 2026 at 06:25:06PM +0300, Amit Barzilai wrote:
> The SSD1351 support was added to the ssd130x DRM driver. To avoid
> confusion and irrelevant updates, the staging fb_ssd1351 driver is
> removed.
>
> Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
fbtft uses a different dt binding iirc, and has a different uABI, so we
should probably keep it around.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply
* Re: [PATCH v1] fbdev: vga16fb: Drop unused assignment of platform_device_id driver data
From: Helge Deller @ 2026-06-23 7:28 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Ard Biesheuvel, Javier Garcia, Vivek BalachandharTN,
Thomas Zimmermann, linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <02710e01b80c06230f399afa6a7d247b95b3f2de.1781689394.git.u.kleine-koenig@baylibre.com>
On 6/17/26 11:44, Uwe Kleine-König (The Capable Hub) wrote:
> The driver explicitly sets the .driver_data member of struct
> platform_device_id to zero without relying on that value. Drop these
> unused assignments.
>
> While touching this array unify spacing and usage of commas and use
> named initializers for .name.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> ---
> drivers/video/fbdev/vga16fb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH] fbdev: fix use-after-free of fb_display[i].mode in store_modes()
From: Ian Bridges @ 2026-06-22 15:40 UTC (permalink / raw)
To: sashiko-reviews
Cc: Simona Vetter, Helge Deller, linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <20260622080749.D7FC61F000E9@smtp.kernel.org>
On Mon, Jun 22, 2026 at 08:07:49AM +0000, sashiko-bot@kernel.org wrote:
I'm new to submitting kernel patches, so please correct me if I have
handled this wrong. All four findings are pre-existing and none are
introduced by this patch, which I scoped to the reported
fb_display[i].mode use-after-free. Responses inline.
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Use-After-Free in show_mode() due to dangling fb_info->mode pointer.
> - [High] Data Race and potential Use-After-Free / list corruption when iterating fb_info->modelist.
> - [High] NULL pointer dereference in fbcon_new_modelist() for unallocated consoles.
> - [Medium] Data Race and locking violation modifying fb_display in do_unregister_framebuffer().
> --
>
> commit 5e256dfc1372231ec16feb6ee8a3218859b0dd4f
> Author: Ian Bridges <icb@fastmail.org>
>
> fbdev: fix use-after-free of fb_display[i].mode in store_modes()
>
> store_modes() replaces a framebuffer's modelist and frees the old entries
> with fb_destroy_modelist(), but fb_display[i].mode was left dangling.
> This commit fixes the use-after-free by clearing the stale pointers with
> fbcon_delete_modelist() before freeing the old list.
>
> Closes: https://syzkaller.appspot.com/bug?extid=81c7c6b52649fd07299d
>
> > diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c
> > index baa2bae0fb5b3..c761dcf21dafb 100644
> > --- a/drivers/video/fbdev/core/fbsysfs.c
> > +++ b/drivers/video/fbdev/core/fbsysfs.c
> > @@ -111,8 +111,10 @@ static ssize_t store_modes(struct device *device,
> > if (fb_new_modelist(fb_info)) {
>
> [Severity: High]
> This isn't a bug introduced by this patch, but can this fb_new_modelist()
> call lead to a NULL pointer dereference for unallocated consoles?
>
> If an unallocated console (where vc_cons[i].d is NULL) is mapped to a
> framebuffer, fbcon_set_disp() populates fb_display[i].mode and returns early.
>
> When store_modes() calls fb_new_modelist() -> fbcon_new_modelist(),
> it checks if con2fb_map and fb_display[i].mode are set. Could it then fetch
> vc = vc_cons[i].d and unconditionally dereference vc->vc_num?
>
> drivers/video/fbdev/core/fbcon.c:fbcon_new_modelist() {
> ...
> vc = vc_cons[i].d;
> ...
> fbcon_set_disp(info, &var, vc->vc_num);
> ...
> }
Confirmed. fbcon_new_modelist() dereferences vc_cons[i].d without a NULL
check, and a console can have con2fb_map and fb_display[i].mode set while
vc_cons[i].d is NULL. I hit this while writing the reproducer for this bug.
It is a separate issue, so I'd argue it needs its own patch adding the NULL
check.
>
> > fb_destroy_modelist(&fb_info->modelist);
> > list_splice(&old_list, &fb_info->modelist);
> > - } else
> > + } else {
> > + fbcon_delete_modelist(&old_list);
>
> [Severity: Medium]
> This is a pre-existing issue, but since we are properly calling
> fbcon_delete_modelist() under console_lock() here, should other callers do
> the same?
>
> Looking at do_unregister_framebuffer(), it executes without holding
> console_lock():
>
> drivers/video/fbdev/core/fbmem.c:do_unregister_framebuffer() {
> ...
> fbcon_delete_modelist(&fb_info->modelist);
> ...
> }
Agreed. do_unregister_framebuffer() runs under registration_lock, not
console_lock, so the fbcon_delete_modelist() call added by a1f305893074
writes fb_display[] without console_lock. The call this patch adds to
store_modes() runs under console_lock and is safe. The unregister call site
looks worth a separate fix.
>
> Since fbcon_delete_mode() iterates the global fb_display array and locklessly
> assigns NULL to fb_display[i].mode, could this cause a data race with
> concurrent readers like FBIOPUT_VSCREENINFO?
>
> > fb_destroy_modelist(&old_list);
>
> [Severity: High]
> This is a pre-existing issue, but does destroying old_list leave
> fb_info->mode dangling?
>
> If fb_info->mode was previously set to point into a struct fb_modelist
> element from this list, it doesn't appear to be cleared before the list
> is freed.
>
> Could a subsequent read of the sysfs mode attribute trigger a use-after-free
> here:
>
> drivers/video/fbdev/core/fbsysfs.c:show_mode() {
> ...
> return mode_string(buf, 0, fb_info->mode);
> }
Confirmed, and it still reproduces with this patch applied. store_modes()
frees the old modelist that fb_info->mode points into, and this patch only
clears fb_display[i].mode. Writing a mode to /sys/class/graphics/fb0/mode,
then a new list to /sys/class/graphics/fb0/modes, then reading
/sys/class/graphics/fb0/mode faults in mode_string() called from
show_mode(). Since it is the same free in store_modes(), I will fold the
fix into a v2 of this patch and clear fb_info->mode alongside
fb_display[i].mode.
>
> [Severity: High]
> This is also a pre-existing issue, but does modifying fb_info->modelist
> under lock_fb_info() race with sysfs readers?
>
> Looking at show_modes() and store_mode(), they appear to iterate
> fb_info->modelist locklessly:
>
> drivers/video/fbdev/core/fbsysfs.c:show_modes() {
> ...
> list_for_each_entry(modelist, &fb_info->modelist, list) {
> ...
> }
Agreed. show_modes() and store_mode() walk fb_info->modelist without
lock_fb_info(), so they can race the splice and free in store_modes(). This
looks like a separate locking fix.
>
> Since store_modes() splices, modifies, and destroys this list, could a
> concurrent sysfs read/write follow corrupted pointers into freed memory?
>
> > + }
> >
> > unlock_fb_info(fb_info);
> > console_unlock();
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/ajjoDhAi2y4ArSlz@dev?part=1
To summarise, I will send a v2 of this patch that clears both stale pointers
in store_modes() (fb_display[i].mode and fb_info->mode), and keep the
fbcon_new_modelist() NULL check and the two locking issues as separate
patches.
Thanks,
Ian
^ permalink raw reply
* [PATCH v2 4/4] staging: fbtft: remove fb_ssd1351 driver
From: Amit Barzilai @ 2026-06-22 15:25 UTC (permalink / raw)
To: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller
Cc: azuddinadam, chintanlike, dri-devel, devicetree, linux-kernel,
linux-fbdev, linux-staging, Amit Barzilai
In-Reply-To: <20260622152506.78627-1-amit.barzilai22@gmail.com>
The SSD1351 support was added to the ssd130x DRM driver. To avoid
confusion and irrelevant updates, the staging fb_ssd1351 driver is
removed.
Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
---
drivers/staging/fbtft/Kconfig | 5 -
drivers/staging/fbtft/Makefile | 1 -
drivers/staging/fbtft/fb_ssd1351.c | 240 -----------------------------
3 files changed, 246 deletions(-)
delete mode 100644 drivers/staging/fbtft/fb_ssd1351.c
diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 92943564cb91..c8ea1a0f3fb0 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -133,11 +133,6 @@ config FB_TFT_SSD1331
help
Framebuffer support for SSD1331
-config FB_TFT_SSD1351
- tristate "FB driver for the SSD1351 LCD Controller"
- help
- Framebuffer support for SSD1351
-
config FB_TFT_ST7735R
tristate "FB driver for the ST7735R LCD Controller"
help
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index e9cdf0f0a7da..c230bf008fc7 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -28,7 +28,6 @@ obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1305.o
obj-$(CONFIG_FB_TFT_SSD1306) += fb_ssd1306.o
obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1325.o
obj-$(CONFIG_FB_TFT_SSD1331) += fb_ssd1331.o
-obj-$(CONFIG_FB_TFT_SSD1351) += fb_ssd1351.o
obj-$(CONFIG_FB_TFT_ST7735R) += fb_st7735r.o
obj-$(CONFIG_FB_TFT_ST7789V) += fb_st7789v.o
obj-$(CONFIG_FB_TFT_TINYLCD) += fb_tinylcd.o
diff --git a/drivers/staging/fbtft/fb_ssd1351.c b/drivers/staging/fbtft/fb_ssd1351.c
deleted file mode 100644
index 6736b09b2f45..000000000000
--- a/drivers/staging/fbtft/fb_ssd1351.c
+++ /dev/null
@@ -1,240 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-#include <linux/backlight.h>
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/spi/spi.h>
-#include <linux/delay.h>
-#include <linux/string_choices.h>
-
-#include "fbtft.h"
-
-#define DRVNAME "fb_ssd1351"
-#define WIDTH 128
-#define HEIGHT 128
-#define GAMMA_NUM 1
-#define GAMMA_LEN 63
-#define DEFAULT_GAMMA "0 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2" \
-
-static void register_onboard_backlight(struct fbtft_par *par);
-
-static int init_display(struct fbtft_par *par)
-{
- if (par->pdata &&
- par->pdata->display.backlight == FBTFT_ONBOARD_BACKLIGHT) {
- /* module uses onboard GPIO for panel power */
- par->fbtftops.register_backlight = register_onboard_backlight;
- }
-
- par->fbtftops.reset(par);
-
- write_reg(par, 0xfd, 0x12); /* Command Lock */
- write_reg(par, 0xfd, 0xb1); /* Command Lock */
- write_reg(par, 0xae); /* Display Off */
- write_reg(par, 0xb3, 0xf1); /* Front Clock Div */
- write_reg(par, 0xca, 0x7f); /* Set Mux Ratio */
- write_reg(par, 0x15, 0x00, 0x7f); /* Set Column Address */
- write_reg(par, 0x75, 0x00, 0x7f); /* Set Row Address */
- write_reg(par, 0xa1, 0x00); /* Set Display Start Line */
- write_reg(par, 0xa2, 0x00); /* Set Display Offset */
- write_reg(par, 0xb5, 0x00); /* Set GPIO */
- write_reg(par, 0xab, 0x01); /* Set Function Selection */
- write_reg(par, 0xb1, 0x32); /* Set Phase Length */
- write_reg(par, 0xb4, 0xa0, 0xb5, 0x55); /* Set Segment Low Voltage */
- write_reg(par, 0xbb, 0x17); /* Set Precharge Voltage */
- write_reg(par, 0xbe, 0x05); /* Set VComH Voltage */
- write_reg(par, 0xc1, 0xc8, 0x80, 0xc8); /* Set Contrast */
- write_reg(par, 0xc7, 0x0f); /* Set Master Contrast */
- write_reg(par, 0xb6, 0x01); /* Set Second Precharge Period */
- write_reg(par, 0xa6); /* Set Display Mode Reset */
- write_reg(par, 0xaf); /* Set Sleep Mode Display On */
-
- return 0;
-}
-
-static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
-{
- write_reg(par, 0x15, xs, xe);
- write_reg(par, 0x75, ys, ye);
- write_reg(par, 0x5c);
-}
-
-static int set_var(struct fbtft_par *par)
-{
- unsigned int remap;
-
- if (par->fbtftops.init_display != init_display) {
- /* don't risk messing up register A0h */
- return 0;
- }
-
- remap = 0x60 | (par->bgr << 2); /* Set Colour Depth */
-
- switch (par->info->var.rotate) {
- case 0:
- write_reg(par, 0xA0, remap | 0x00 | BIT(4));
- break;
- case 270:
- write_reg(par, 0xA0, remap | 0x03 | BIT(4));
- break;
- case 180:
- write_reg(par, 0xA0, remap | 0x02);
- break;
- case 90:
- write_reg(par, 0xA0, remap | 0x01);
- break;
- }
-
- return 0;
-}
-
-/*
- * Grayscale Lookup Table
- * GS1 - GS63
- * The driver Gamma curve contains the relative values between the entries
- * in the Lookup table.
- *
- * From datasheet:
- * 8.8 Gray Scale Decoder
- *
- * there are total 180 Gamma Settings (Setting 0 to Setting 180)
- * available for the Gray Scale table.
- *
- * The gray scale is defined in incremental way, with reference
- * to the length of previous table entry:
- * Setting of GS1 has to be >= 0
- * Setting of GS2 has to be > Setting of GS1 +1
- * Setting of GS3 has to be > Setting of GS2 +1
- * :
- * Setting of GS63 has to be > Setting of GS62 +1
- *
- */
-static int set_gamma(struct fbtft_par *par, u32 *curves)
-{
- unsigned long tmp[GAMMA_NUM * GAMMA_LEN];
- int i, acc = 0;
-
- for (i = 0; i < 63; i++) {
- if (i > 0 && curves[i] < 2) {
- dev_err(par->info->device,
- "Illegal value in Grayscale Lookup Table at index %d : %d. Must be greater than 1\n",
- i, curves[i]);
- return -EINVAL;
- }
- acc += curves[i];
- tmp[i] = acc;
- if (acc > 180) {
- dev_err(par->info->device,
- "Illegal value(s) in Grayscale Lookup Table. At index=%d : %d, the accumulated value has exceeded 180\n",
- i, acc);
- return -EINVAL;
- }
- }
-
- write_reg(par, 0xB8,
- tmp[0], tmp[1], tmp[2], tmp[3],
- tmp[4], tmp[5], tmp[6], tmp[7],
- tmp[8], tmp[9], tmp[10], tmp[11],
- tmp[12], tmp[13], tmp[14], tmp[15],
- tmp[16], tmp[17], tmp[18], tmp[19],
- tmp[20], tmp[21], tmp[22], tmp[23],
- tmp[24], tmp[25], tmp[26], tmp[27],
- tmp[28], tmp[29], tmp[30], tmp[31],
- tmp[32], tmp[33], tmp[34], tmp[35],
- tmp[36], tmp[37], tmp[38], tmp[39],
- tmp[40], tmp[41], tmp[42], tmp[43],
- tmp[44], tmp[45], tmp[46], tmp[47],
- tmp[48], tmp[49], tmp[50], tmp[51],
- tmp[52], tmp[53], tmp[54], tmp[55],
- tmp[56], tmp[57], tmp[58], tmp[59],
- tmp[60], tmp[61], tmp[62]);
-
- return 0;
-}
-
-static int blank(struct fbtft_par *par, bool on)
-{
- fbtft_par_dbg(DEBUG_BLANK, par, "(%s=%s)\n",
- __func__, str_true_false(on));
- if (on)
- write_reg(par, 0xAE);
- else
- write_reg(par, 0xAF);
- return 0;
-}
-
-static struct fbtft_display display = {
- .regwidth = 8,
- .width = WIDTH,
- .height = HEIGHT,
- .gamma_num = GAMMA_NUM,
- .gamma_len = GAMMA_LEN,
- .gamma = DEFAULT_GAMMA,
- .fbtftops = {
- .init_display = init_display,
- .set_addr_win = set_addr_win,
- .set_var = set_var,
- .set_gamma = set_gamma,
- .blank = blank,
- },
-};
-
-static int update_onboard_backlight(struct backlight_device *bd)
-{
- struct fbtft_par *par = bl_get_data(bd);
- bool on;
-
- fbtft_par_dbg(DEBUG_BACKLIGHT, par, "%s: power=%d\n", __func__, bd->props.power);
-
- on = !backlight_is_blank(bd);
- /* Onboard backlight connected to GPIO0 on SSD1351, GPIO1 unused */
- write_reg(par, 0xB5, on ? 0x03 : 0x02);
-
- return 0;
-}
-
-static const struct backlight_ops bl_ops = {
- .update_status = update_onboard_backlight,
-};
-
-static void register_onboard_backlight(struct fbtft_par *par)
-{
- struct backlight_device *bd;
- struct backlight_properties bl_props = { 0, };
-
- bl_props.type = BACKLIGHT_RAW;
- bl_props.power = BACKLIGHT_POWER_OFF;
-
- bd = backlight_device_register(dev_driver_string(par->info->device),
- par->info->device, par, &bl_ops,
- &bl_props);
- if (IS_ERR(bd)) {
- dev_err(par->info->device,
- "cannot register backlight device (%ld)\n",
- PTR_ERR(bd));
- return;
- }
- par->info->bl_dev = bd;
-
- if (!par->fbtftops.unregister_backlight)
- par->fbtftops.unregister_backlight = fbtft_unregister_backlight;
-}
-
-FBTFT_REGISTER_DRIVER(DRVNAME, "solomon,ssd1351", &display);
-
-MODULE_ALIAS("spi:" DRVNAME);
-MODULE_ALIAS("platform:" DRVNAME);
-MODULE_ALIAS("spi:ssd1351");
-MODULE_ALIAS("platform:ssd1351");
-
-MODULE_DESCRIPTION("SSD1351 OLED Driver");
-MODULE_AUTHOR("James Davies");
-MODULE_LICENSE("GPL");
--
2.54.0
^ permalink raw reply related
* [PATCH v2 3/4] drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support
From: Amit Barzilai @ 2026-06-22 15:25 UTC (permalink / raw)
To: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller
Cc: azuddinadam, chintanlike, dri-devel, devicetree, linux-kernel,
linux-fbdev, linux-staging, Amit Barzilai
In-Reply-To: <20260622152506.78627-1-amit.barzilai22@gmail.com>
The Solomon SSD1351 is a 128x128 RGB color OLED controller. It shares
the SSD133X data path: a column/row addressing window followed by a bulk
RGB565 pixel write. Add it as a new SSD135X_FAMILY rather than a separate
driver, reusing the SSD133X plane, CRTC and blit/clear helpers.
The only data-path difference is that the SSD1351 requires an explicit
Write RAM command (0x5c) after the address window is programmed, before
pixel data is accepted, whereas the SSD133X enters data mode implicitly.
This is emitted from a shared ssd133x_write_pixels() helper so both the
damage-update and clear-screen paths cover it.
The SSD1351 also needs its own init sequence (ssd135x_init), dispatched
via ssd135x_encoder_atomic_enable, and a longer post-reset settle delay.
The re-map byte is fixed at 0 degrees, 65k color, COM split, BGR
sub-pixel order; rotation is not supported.
The SSD1351 is SPI-only, so only the SPI transport match tables gain an
entry; no new config symbol is needed.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
---
drivers/gpu/drm/solomon/ssd130x-spi.c | 7 +
drivers/gpu/drm/solomon/ssd130x.c | 214 +++++++++++++++++++++-----
drivers/gpu/drm/solomon/ssd130x.h | 5 +-
3 files changed, 189 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd130x-spi.c b/drivers/gpu/drm/solomon/ssd130x-spi.c
index b52f5fd592a1..6e0dd6e5a88d 100644
--- a/drivers/gpu/drm/solomon/ssd130x-spi.c
+++ b/drivers/gpu/drm/solomon/ssd130x-spi.c
@@ -146,6 +146,11 @@ static const struct of_device_id ssd130x_of_match[] = {
.compatible = "solomon,ssd1331",
.data = &ssd130x_variants[SSD1331_ID],
},
+ /* ssd135x family */
+ {
+ .compatible = "solomon,ssd1351",
+ .data = &ssd130x_variants[SSD1351_ID],
+ },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, ssd130x_of_match);
@@ -171,6 +176,8 @@ static const struct spi_device_id ssd130x_spi_id[] = {
{ "ssd1327", SSD1327_ID },
/* ssd133x family */
{ "ssd1331", SSD1331_ID },
+ /* ssd135x family */
+ { "ssd1351", SSD1351_ID },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(spi, ssd130x_spi_id);
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index 2b0a8218f529..e5a9428f91b8 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -146,6 +146,33 @@
#define SSD133X_COLOR_DEPTH_256 0x0
#define SSD133X_COLOR_DEPTH_65K 0x1
+/* ssd135x commands */
+#define SSD135X_SET_COL_RANGE 0x15
+#define SSD135X_WRITE_RAM 0x5c
+#define SSD135X_SET_ROW_RANGE 0x75
+#define SSD135X_SET_DISPLAY_START 0xa1
+#define SSD135X_SET_DISPLAY_OFFSET 0xa2
+#define SSD135X_SET_DISPLAY_NORMAL 0xa6
+#define SSD135X_SET_FUNCTION 0xab
+#define SSD135X_SET_PHASE_LENGTH 0xb1
+#define SSD135X_SET_CLOCK_FREQ 0xb3
+#define SSD135X_SET_VSL 0xb4
+#define SSD135X_SET_GPIO 0xb5
+#define SSD135X_SET_PRECHARGE2 0xb6
+#define SSD135X_SET_PRECHARGE 0xbb
+#define SSD135X_SET_VCOMH_VOLTAGE 0xbe
+#define SSD135X_SET_CONTRAST 0xc1
+#define SSD135X_SET_CONTRAST_MASTER 0xc7
+#define SSD135X_SET_MUX_RATIO 0xca
+#define SSD135X_SET_COMMAND_LOCK 0xfd
+
+/* ssd135x remap byte (data of SSD13XX_SET_SEG_REMAP) */
+#define SSD135X_SET_REMAP_COLUMN BIT(1)
+#define SSD135X_SET_REMAP_COLOR_BGR BIT(2)
+#define SSD135X_SET_REMAP_COM_SCAN BIT(4)
+#define SSD135X_SET_REMAP_COM_SPLIT BIT(5)
+#define SSD135X_SET_REMAP_65K BIT(6)
+
#define MAX_CONTRAST 255
const struct ssd130x_deviceinfo ssd130x_variants[] = {
@@ -214,6 +241,13 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = {
.default_height = 64,
.format_rgb565 = 1,
.family_id = SSD133X_FAMILY,
+ },
+ /* ssd135x family */
+ [SSD1351_ID] = {
+ .default_width = 128,
+ .default_height = 128,
+ .format_rgb565 = 1,
+ .family_id = SSD135X_FAMILY,
}
};
EXPORT_SYMBOL_NS_GPL(ssd130x_variants, "DRM_SSD130X");
@@ -248,47 +282,16 @@ static inline struct ssd130x_device *drm_to_ssd130x(struct drm_device *drm)
/*
* Helper to write data (SSD13XX_DATA) to the device.
*/
-static int ssd130x_write_data(struct ssd130x_device *ssd130x, u8 *values, int count)
+static int ssd130x_write_data(struct ssd130x_device *ssd130x, const u8 *values, int count)
{
return regmap_bulk_write(ssd130x->regmap, SSD13XX_DATA, values, count);
}
-/*
- * Helper to write command (SSD13XX_COMMAND). The fist variadic argument
- * is the command to write and the following are the command options.
- *
- * Note that the ssd13xx protocol requires each command and option to be
- * written as a SSD13XX_COMMAND device register value. That is why a call
- * to regmap_write(..., SSD13XX_COMMAND, ...) is done for each argument.
- */
-static int ssd130x_write_cmd(struct ssd130x_device *ssd130x, int count,
- /* u8 cmd, u8 option, ... */...)
-{
- va_list ap;
- u8 value;
- int ret;
-
- va_start(ap, count);
-
- do {
- value = va_arg(ap, int);
- ret = regmap_write(ssd130x->regmap, SSD13XX_COMMAND, value);
- if (ret)
- goto out_end;
- } while (--count);
-
-out_end:
- va_end(ap);
-
- return ret;
-}
-
/*
* Write a command byte sequence from a buffer.
*
- * Like ssd130x_write_cmd() but takes a pre-built byte array instead of
- * variadic arguments, handy when the command is already in an array or
- * when the caller wants to use sizeof() for the length.
+ * The first byte is the command opcode and the following bytes are its
+ * options/parameters.
*/
static int ssd130x_write_cmds(struct ssd130x_device *ssd130x, const u8 *cmd,
size_t len)
@@ -296,6 +299,22 @@ static int ssd130x_write_cmds(struct ssd130x_device *ssd130x, const u8 *cmd,
unsigned int i;
int ret;
+ /*
+ * The SSD135X family latches command parameters with D/C# HIGH (i.e.
+ * clocked in as data), unlike the other families where the opcode and
+ * all of its parameters are sent as commands (D/C# LOW). Send the
+ * opcode as a command and any following parameter bytes as data.
+ */
+ if (ssd130x->device_info->family_id == SSD135X_FAMILY) {
+ if (len == 0)
+ return 0;
+ ret = regmap_write(ssd130x->regmap, SSD13XX_COMMAND, cmd[0]);
+ if (ret || len == 1)
+ return ret;
+
+ return ssd130x_write_data(ssd130x, cmd + 1, len - 1);
+ }
+
for (i = 0; i < len; i++) {
ret = regmap_write(ssd130x->regmap, SSD13XX_COMMAND, cmd[i]);
if (ret)
@@ -305,6 +324,28 @@ static int ssd130x_write_cmds(struct ssd130x_device *ssd130x, const u8 *cmd,
return 0;
}
+/*
+ * Variadic wrapper around ssd130x_write_cmds(). The first variadic argument is
+ * the command opcode and the following ones are its options/parameters.
+ */
+static int ssd130x_write_cmd(struct ssd130x_device *ssd130x, int count,
+ /* u8 cmd, u8 option, ... */...)
+{
+ u8 buf[8];
+ va_list ap;
+ int i;
+
+ if (count > ARRAY_SIZE(buf))
+ return -EINVAL;
+
+ va_start(ap, count);
+ for (i = 0; i < count; i++)
+ buf[i] = va_arg(ap, int);
+ va_end(ap);
+
+ return ssd130x_write_cmds(ssd130x, buf, count);
+}
+
/*
* Run a packed command sequence. The format is a flat byte array where each
* entry starts with a length byte followed by that many command bytes. A
@@ -628,6 +669,49 @@ static int ssd133x_init(struct ssd130x_device *ssd130x)
return ssd130x_run_cmd_seq(ssd130x, cmds);
}
+static int ssd135x_init(struct ssd130x_device *ssd130x)
+{
+ /*
+ * Horizontal address increment, COM split, reversed COM scan direction,
+ * BGR sub-pixel order and 65k (RGB565) color depth. Rotation is not
+ * supported, so the remap byte is fixed.
+ */
+ u8 remap = SSD135X_SET_REMAP_65K | SSD135X_SET_REMAP_COM_SPLIT |
+ SSD135X_SET_REMAP_COLOR_BGR | SSD135X_SET_REMAP_COM_SCAN;
+ const u8 cmds[] = {
+ 2, SSD135X_SET_COMMAND_LOCK, 0x12,
+ 2, SSD135X_SET_COMMAND_LOCK, 0xb1,
+ 1, SSD13XX_DISPLAY_OFF,
+ 2, SSD135X_SET_CLOCK_FREQ, 0xf1,
+ 2, SSD135X_SET_MUX_RATIO, ssd130x->height - 1,
+ 3, SSD135X_SET_COL_RANGE, 0x00, ssd130x->width - 1,
+ 3, SSD135X_SET_ROW_RANGE, 0x00, ssd130x->height - 1,
+ 2, SSD135X_SET_DISPLAY_START, 0x00,
+ 2, SSD135X_SET_DISPLAY_OFFSET, 0x00,
+ 2, SSD135X_SET_GPIO, 0x00,
+ 2, SSD135X_SET_FUNCTION, 0x01,
+ 2, SSD135X_SET_PHASE_LENGTH, 0x32,
+ 4, SSD135X_SET_VSL, 0xa0, 0xb5, 0x55,
+ 2, SSD135X_SET_PRECHARGE, 0x17,
+ 2, SSD135X_SET_VCOMH_VOLTAGE, 0x05,
+ 4, SSD135X_SET_CONTRAST, 0xc8, 0x80, 0xc8,
+ 2, SSD135X_SET_CONTRAST_MASTER, 0x0f,
+ 2, SSD135X_SET_PRECHARGE2, 0x01,
+ 1, SSD135X_SET_DISPLAY_NORMAL,
+ 2, SSD13XX_SET_SEG_REMAP, remap,
+ 0,
+ };
+
+ /*
+ * ssd130x_power_on() issues a short reset pulse, but the SSD1351 is not
+ * ready to accept commands immediately afterwards. Give the controller
+ * time to settle before sending the init sequence.
+ */
+ msleep(120);
+
+ return ssd130x_run_cmd_seq(ssd130x, cmds);
+}
+
static int ssd130x_update_rect(struct ssd130x_device *ssd130x,
struct drm_rect *rect, u8 *buf,
u8 *data_array)
@@ -790,6 +874,25 @@ static int ssd132x_update_rect(struct ssd130x_device *ssd130x,
return ret;
}
+/*
+ * Write a run of pixel data to the controller's display RAM. The SSD135X
+ * family requires an explicit Write RAM command once the address window has
+ * been set, before any pixel data is accepted; the SSD133X family enters data
+ * mode implicitly after the column/row range is programmed.
+ */
+static int ssd133x_write_pixels(struct ssd130x_device *ssd130x,
+ u8 *data_array, unsigned int count)
+{
+ if (ssd130x->device_info->family_id == SSD135X_FAMILY) {
+ int ret = ssd130x_write_cmd(ssd130x, 1, SSD135X_WRITE_RAM);
+
+ if (ret < 0)
+ return ret;
+ }
+
+ return ssd130x_write_data(ssd130x, data_array, count);
+}
+
static int ssd133x_update_rect(struct ssd130x_device *ssd130x,
struct drm_rect *rect, u8 *data_array,
unsigned int pitch)
@@ -832,7 +935,7 @@ static int ssd133x_update_rect(struct ssd130x_device *ssd130x,
return ret;
/* Write out update in one go since horizontal addressing mode is used */
- ret = ssd130x_write_data(ssd130x, data_array, pitch * rows);
+ ret = ssd133x_write_pixels(ssd130x, data_array, pitch * rows);
return ret;
}
@@ -917,7 +1020,7 @@ static void ssd133x_clear_screen(struct ssd130x_device *ssd130x, u8 *data_array)
memset(data_array, 0, pitch * ssd130x->height);
/* Write out update in one go since horizontal addressing mode is used */
- ssd130x_write_data(ssd130x, data_array, pitch * ssd130x->height);
+ ssd133x_write_pixels(ssd130x, data_array, pitch * ssd130x->height);
}
static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb,
@@ -1380,6 +1483,12 @@ static const struct drm_plane_helper_funcs ssd130x_primary_plane_helper_funcs[]
.atomic_check = ssd133x_primary_plane_atomic_check,
.atomic_update = ssd133x_primary_plane_atomic_update,
.atomic_disable = ssd133x_primary_plane_atomic_disable,
+ },
+ [SSD135X_FAMILY] = {
+ DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
+ .atomic_check = ssd133x_primary_plane_atomic_check,
+ .atomic_update = ssd133x_primary_plane_atomic_update,
+ .atomic_disable = ssd133x_primary_plane_atomic_disable,
}
};
@@ -1534,6 +1643,10 @@ static const struct drm_crtc_helper_funcs ssd130x_crtc_helper_funcs[] = {
.mode_valid = ssd130x_crtc_mode_valid,
.atomic_check = ssd133x_crtc_atomic_check,
},
+ [SSD135X_FAMILY] = {
+ .mode_valid = ssd130x_crtc_mode_valid,
+ .atomic_check = ssd133x_crtc_atomic_check,
+ },
};
static const struct drm_crtc_funcs ssd130x_crtc_funcs = {
@@ -1621,6 +1734,31 @@ static void ssd133x_encoder_atomic_enable(struct drm_encoder *encoder,
ssd130x_power_off(ssd130x);
}
+static void ssd135x_encoder_atomic_enable(struct drm_encoder *encoder,
+ struct drm_atomic_commit *state)
+{
+ struct drm_device *drm = encoder->dev;
+ struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
+ int ret;
+
+ ret = ssd130x_power_on(ssd130x);
+ if (ret)
+ return;
+
+ ret = ssd135x_init(ssd130x);
+ if (ret)
+ goto power_off;
+
+ ssd130x_write_cmd(ssd130x, 1, SSD13XX_DISPLAY_ON);
+
+ backlight_enable(ssd130x->bl_dev);
+
+ return;
+
+power_off:
+ ssd130x_power_off(ssd130x);
+}
+
static void ssd130x_encoder_atomic_disable(struct drm_encoder *encoder,
struct drm_atomic_commit *state)
{
@@ -1646,6 +1784,10 @@ static const struct drm_encoder_helper_funcs ssd130x_encoder_helper_funcs[] = {
[SSD133X_FAMILY] = {
.atomic_enable = ssd133x_encoder_atomic_enable,
.atomic_disable = ssd130x_encoder_atomic_disable,
+ },
+ [SSD135X_FAMILY] = {
+ .atomic_enable = ssd135x_encoder_atomic_enable,
+ .atomic_disable = ssd130x_encoder_atomic_disable,
}
};
diff --git a/drivers/gpu/drm/solomon/ssd130x.h b/drivers/gpu/drm/solomon/ssd130x.h
index b0b487c06e04..da89d4455270 100644
--- a/drivers/gpu/drm/solomon/ssd130x.h
+++ b/drivers/gpu/drm/solomon/ssd130x.h
@@ -26,7 +26,8 @@
enum ssd130x_family_ids {
SSD130X_FAMILY,
SSD132X_FAMILY,
- SSD133X_FAMILY
+ SSD133X_FAMILY,
+ SSD135X_FAMILY
};
enum ssd130x_variants {
@@ -42,6 +43,8 @@ enum ssd130x_variants {
SSD1327_ID,
/* ssd133x family */
SSD1331_ID,
+ /* ssd135x family */
+ SSD1351_ID,
NR_SSD130X_VARIANTS
};
--
2.54.0
^ permalink raw reply related
* [PATCH v2 2/4] drm/ssd130x: Add RGB565 support to SSD133X family
From: Amit Barzilai @ 2026-06-22 15:25 UTC (permalink / raw)
To: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller
Cc: azuddinadam, chintanlike, dri-devel, devicetree, linux-kernel,
linux-fbdev, linux-staging, Amit Barzilai
In-Reply-To: <20260622152506.78627-1-amit.barzilai22@gmail.com>
SSD133X screens were getting 8bpp (RGB332) instead of the 16bpp
(RGB565) that they support. This change adds a boolean to the
deviceinfo struct selecting whether the variant is driven at
DRM_FORMAT_RGB565.
Changed SSD133X to now utilize 65k color (RGB565).
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
---
drivers/gpu/drm/solomon/ssd130x.c | 55 +++++++++++++++++++++++++------
drivers/gpu/drm/solomon/ssd130x.h | 7 ++++
2 files changed, 52 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index 04da4f2f7d08..2b0a8218f529 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -140,6 +140,12 @@
#define SSD133X_SET_PRECHARGE_VOLTAGE 0xbb
#define SSD133X_SET_VCOMH_VOLTAGE 0xbe
+/* ssd133x remap byte (data of SSD13XX_SET_SEG_REMAP) */
+#define SSD133X_SET_REMAP_COM_SPLIT BIT(5)
+#define SSD133X_SET_REMAP_COLOR_DEPTH_MASK GENMASK(7, 6)
+#define SSD133X_COLOR_DEPTH_256 0x0
+#define SSD133X_COLOR_DEPTH_65K 0x1
+
#define MAX_CONTRAST 255
const struct ssd130x_deviceinfo ssd130x_variants[] = {
@@ -206,6 +212,7 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = {
[SSD1331_ID] = {
.default_width = 96,
.default_height = 64,
+ .format_rgb565 = 1,
.family_id = SSD133X_FAMILY,
}
};
@@ -584,6 +591,10 @@ static int ssd132x_init(struct ssd130x_device *ssd130x)
static int ssd133x_init(struct ssd130x_device *ssd130x)
{
+ u8 remap = SSD133X_SET_REMAP_COM_SPLIT |
+ FIELD_PREP(SSD133X_SET_REMAP_COLOR_DEPTH_MASK,
+ ssd130x->device_info->format_rgb565 ?
+ SSD133X_COLOR_DEPTH_65K : SSD133X_COLOR_DEPTH_256);
const u8 cmds[] = {
2, SSD133X_CONTRAST_A, 0x91,
2, SSD133X_CONTRAST_B, 0x50,
@@ -595,9 +606,9 @@ static int ssd133x_init(struct ssd130x_device *ssd130x)
* Horizontal Address Increment
* Normal order SA,SB,SC (e.g. RGB)
* COM Split Odd Even
- * 256 color format
+ * 256 or 65k color format, depending on the variant
*/
- 2, SSD13XX_SET_SEG_REMAP, 0x20,
+ 2, SSD13XX_SET_SEG_REMAP, remap,
2, SSD133X_SET_DISPLAY_START, 0x00,
2, SSD133X_SET_DISPLAY_OFFSET, 0x00,
1, SSD133X_SET_DISPLAY_NORMAL,
@@ -794,14 +805,20 @@ static int ssd133x_update_rect(struct ssd130x_device *ssd130x,
* COM0 to COM[N - 1] are the rows and SEG0 to SEG[M - 1] are
* the columns.
*
- * Each Segment has a 8-bit pixel and each Common output has a
- * row of pixels. When using the (default) horizontal address
- * increment mode, each byte of data sent to the controller has
- * a Segment (e.g: SEG0).
+ * Each Segment holds one pixel and each Common output has a row
+ * of pixels. A pixel is 8 bits (one byte) in the 256 color
+ * (RGB332) format or 16 bits (two bytes) in the 65k color
+ * (RGB565) format. When using the (default) horizontal address
+ * increment mode, the pixel data is sent Segment by Segment
+ * (e.g: SEG0 first).
*
* When using the 256 color depth format, each pixel contains 3
* sub-pixels for color A, B and C. These have 3 bit, 3 bit and
* 2 bits respectively.
+ *
+ * When using the 65k color depth format, each pixel contains 3
+ * sub-pixels for color A, B and C. These have 5 bit, 6 bit and
+ * 5 bits respectively.
*/
/* Set column start and end */
@@ -872,9 +889,24 @@ static void ssd132x_clear_screen(struct ssd130x_device *ssd130x, u8 *data_array)
ssd130x_write_data(ssd130x, data_array, columns * height);
}
+/*
+ * The SSD133X family can drive the panel in either RGB332 (1 byte per pixel)
+ * or RGB565 (2 bytes per pixel). The format is a per-variant policy choice
+ * selected through ssd130x_deviceinfo::format_rgb565, not a capability probe.
+ * Centralize the choice here so that the buffer sizing (allocation, clear and
+ * blit pitch) can never disagree.
+ */
+static const struct drm_format_info *ssd133x_format_info(struct ssd130x_device *ssd130x)
+{
+ if (ssd130x->device_info->format_rgb565)
+ return drm_format_info(DRM_FORMAT_RGB565);
+
+ return drm_format_info(DRM_FORMAT_RGB332);
+}
+
static void ssd133x_clear_screen(struct ssd130x_device *ssd130x, u8 *data_array)
{
- const struct drm_format_info *fi = drm_format_info(DRM_FORMAT_RGB332);
+ const struct drm_format_info *fi = ssd133x_format_info(ssd130x);
unsigned int pitch;
if (!fi)
@@ -945,7 +977,7 @@ static int ssd133x_fb_blit_rect(struct drm_framebuffer *fb,
struct drm_format_conv_state *fmtcnv_state)
{
struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev);
- const struct drm_format_info *fi = drm_format_info(DRM_FORMAT_RGB332);
+ const struct drm_format_info *fi = ssd133x_format_info(ssd130x);
unsigned int dst_pitch;
struct iosys_map dst;
int ret = 0;
@@ -956,7 +988,10 @@ static int ssd133x_fb_blit_rect(struct drm_framebuffer *fb,
dst_pitch = drm_format_info_min_pitch(fi, 0, drm_rect_width(rect));
iosys_map_set_vaddr(&dst, data_array);
- drm_fb_xrgb8888_to_rgb332(&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state);
+ if (ssd130x->device_info->format_rgb565)
+ drm_fb_xrgb8888_to_rgb565be(&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state);
+ else
+ drm_fb_xrgb8888_to_rgb332(&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state);
ssd133x_update_rect(ssd130x, rect, data_array, dst_pitch);
@@ -1414,7 +1449,7 @@ static int ssd133x_crtc_atomic_check(struct drm_crtc *crtc,
struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
struct ssd130x_crtc_state *ssd130x_state = to_ssd130x_crtc_state(crtc_state);
- const struct drm_format_info *fi = drm_format_info(DRM_FORMAT_RGB332);
+ const struct drm_format_info *fi = ssd133x_format_info(ssd130x);
unsigned int pitch;
int ret;
diff --git a/drivers/gpu/drm/solomon/ssd130x.h b/drivers/gpu/drm/solomon/ssd130x.h
index a4554018bb2a..b0b487c06e04 100644
--- a/drivers/gpu/drm/solomon/ssd130x.h
+++ b/drivers/gpu/drm/solomon/ssd130x.h
@@ -54,6 +54,13 @@ struct ssd130x_deviceinfo {
bool need_pwm;
bool need_chargepump;
bool page_mode_only;
+ /*
+ * Per-variant output format selector for the SSD133X data path. The
+ * hardware can drive the panel in RGB332 (1 byte/pixel) or RGB565
+ * (2 bytes/pixel); this is a policy choice per variant, not a
+ * capability probe. When set, the variant is driven at RGB565.
+ */
+ bool format_rgb565;
enum ssd130x_family_ids family_id;
};
--
2.54.0
^ permalink raw reply related
* [PATCH v2 1/4] dt-bindings: display: Add Solomon SSD1351 OLED controller
From: Amit Barzilai @ 2026-06-22 15:25 UTC (permalink / raw)
To: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller
Cc: azuddinadam, chintanlike, dri-devel, devicetree, linux-kernel,
linux-fbdev, linux-staging, Amit Barzilai
In-Reply-To: <20260622152506.78627-1-amit.barzilai22@gmail.com>
Add a device tree binding for the Solomon SSD1351, a 128x128 65k-color
RGB OLED display controller driven over a 4-wire SPI bus. The binding
builds on the shared solomon,ssd-common.yaml properties already used by
the other Solomon display controllers.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
.../bindings/display/solomon,ssd1351.yaml | 42 +++++++++++++++++++
1 file changed, 42 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd1351.yaml
diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1351.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1351.yaml
new file mode 100644
index 000000000000..80850c2ab5b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/solomon,ssd1351.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/solomon,ssd1351.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Solomon SSD1351 OLED Display Controller
+
+maintainers:
+ - Amit Barzilai <amit.barzilai22@gmail.com>
+ - Javier Martinez Canillas <javierm@redhat.com>
+
+allOf:
+ - $ref: solomon,ssd-common.yaml#
+
+properties:
+ compatible:
+ enum:
+ - solomon,ssd1351
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ oled@0 {
+ compatible = "solomon,ssd1351";
+ reg = <0x0>;
+ reset-gpios = <&gpio2 7 GPIO_ACTIVE_LOW>;
+ dc-gpios = <&gpio2 8 GPIO_ACTIVE_HIGH>;
+ spi-max-frequency = <10000000>;
+ };
+ };
--
2.54.0
^ permalink raw reply related
* [PATCH v2 0/4] drm/ssd130x: Add support for the Solomon SSD1351 OLED controller
From: Amit Barzilai @ 2026-06-22 15:25 UTC (permalink / raw)
To: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller
Cc: azuddinadam, chintanlike, dri-devel, devicetree, linux-kernel,
linux-fbdev, linux-staging, Amit Barzilai
This series adds support for the Solomon SSD1351, a 128x128 65k-color
RGB OLED controller, to the ssd130x DRM driver, and removes the legacy
fbtft fb_ssd1351 driver it supersedes.
v1 [1] was a self-contained ssd1351.c driver. Following Javier's review,
the SSD1351 is instead folded into ssd130x as a new color family, which
also brings 65k color to the existing SSD1331. The work is split as:
- Patch 1 adds the device tree binding. It was previously posted
standalone as a v2 [2]; it is folded into this series here, as Conor
asked, so the binding lands together with the driver and the fbtft
removal.
- Patch 2 changes the SSD133X family to drive RGB565 instead of
RGB332, via a per-variant flag in deviceinfo. The SSD1331 is the
only current member and gains 65k color from this.
- Patch 3 adds the SSD1351 as a new SSD135X_FAMILY, reusing the
SSD133X plane/CRTC and blit/clear helpers. The only data-path
difference is the explicit Write RAM command (0x5c) the SSD1351
needs before pixel data; it also gets its own init sequence and a
longer post-reset settle delay.
- Patch 4 removes the now-redundant staging fbtft fb_ssd1351 driver.
Testing:
- The SSD1351 (patches 1 and 3) is tested on hardware.
- The SSD1331 RGB565 change (patch 2) is compile-tested only; I do not
currently have a working SSD1331 panel. Javier has kindly offered to
test it on his SSD1331.
Dependency:
The SSD1351 reuses ssd133x_update_rect(), which programs the column
and row *end* address as a relative offset rather than an absolute
coordinate. This breaks partial updates that do not start at (0,0). A
separate fix is posted at [3]; until it lands, the SSD1351 shows the
same partial-redraw artifacts. This series applies independently of
that fix, but the two are best merged together.
Based on drm-misc-next.
[1] standalone v1 driver:
https://lore.kernel.org/dri-devel/20260615181253.97551-1-amit.barzilai22@gmail.com
[2] standalone v2 binding:
https://lore.kernel.org/dri-devel/20260615175620.88828-1-amit.barzilai22@gmail.com
[3] ssd132x/ssd133x update_rect end-address fix:
https://lore.kernel.org/dri-devel/20260622122604.32500-1-amit.barzilai22@gmail.com
---
Changes since v1:
- Fold the SSD1351 into ssd130x as a new SSD135X family instead of a
standalone ssd1351.c driver (per Javier).
- Add RGB565 to the SSD133X family, so the SSD1331 also gains 65k color.
- Drop native 256k color (no matching DRM fourcc) and the 0/180
rotation support, to keep the series focused; both can return later.
- Binding: drop solomon,width / solomon,height (deducible from the
compatible) and the rotation property (no consumer), per Krzysztof;
use dt-bindings/gpio/gpio.h flag defines in the example.
- Remove the staging fbtft fb_ssd1351 driver in the same series (per
Conor).
Amit Barzilai (4):
dt-bindings: display: Add Solomon SSD1351 OLED controller
drm/ssd130x: Add RGB565 support to SSD133X family
drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support
staging: fbtft: remove fb_ssd1351 driver
.../bindings/display/solomon,ssd1351.yaml | 42 +++
drivers/gpu/drm/solomon/ssd130x-spi.c | 7 +
drivers/gpu/drm/solomon/ssd130x.c | 269 +++++++++++++++---
drivers/gpu/drm/solomon/ssd130x.h | 12 +-
drivers/staging/fbtft/Kconfig | 5 -
drivers/staging/fbtft/Makefile | 1 -
drivers/staging/fbtft/fb_ssd1351.c | 240 ----------------
7 files changed, 283 insertions(+), 293 deletions(-)
create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd1351.yaml
delete mode 100644 drivers/staging/fbtft/fb_ssd1351.c
--
2.54.0
^ permalink raw reply
* Re: [PATCH] staging: fbtft: fix parenthesis alignment in fb_tinylcd.c
From: Dan Carpenter @ 2026-06-22 9:05 UTC (permalink / raw)
To: Aditya Chari
Cc: andy, gregkh, dri-devel, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <20260621062945.42519-1-adi25charis@gmail.com>
On Sun, Jun 21, 2026 at 11:59:45AM +0530, Aditya Chari wrote:
> Fix a checkpatch.pl
> CHECK:PARENTHESIS_ALIGNMENT warning by aligning the wrapped
> argument list of write_reg() with the line above it.
>
> Signed-off-by: Aditya Chari <adi25charis@gmail.com>
> ---
> drivers/staging/fbtft/fb_tinylcd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/fbtft/fb_tinylcd.c b/drivers/staging/fbtft/fb_tinylcd.c
> index afa8f1c74..d58b12472 100644
> --- a/drivers/staging/fbtft/fb_tinylcd.c
> +++ b/drivers/staging/fbtft/fb_tinylcd.c
> @@ -38,7 +38,7 @@ static int init_display(struct fbtft_par *par)
> write_reg(par, 0xE5, 0x00);
> write_reg(par, 0xF0, 0x36, 0xA5, 0x53);
> write_reg(par, 0xE0, 0x00, 0x35, 0x33, 0x00, 0x00, 0x00,
> - 0x00, 0x35, 0x33, 0x00, 0x00, 0x00);
> + 0x00, 0x35, 0x33, 0x00, 0x00, 0x00);
The original is deliberate. Just leave it as-is, please.
regards,
dan carpenter
^ permalink raw reply
* [PATCH] fbdev: fix use-after-free of fb_display[i].mode in store_modes()
From: Ian Bridges @ 2026-06-22 7:45 UTC (permalink / raw)
To: Simona Vetter, Helge Deller, linux-fbdev, dri-devel, linux-kernel
store_modes() replaces a framebuffer's modelist and frees the old entries
with fb_destroy_modelist(). fb_display[i].mode is a back-pointer into a
modelist entry, and it is not cleared when a console is detached from the
framebuffer (unbinding fbcon clears con2fb_map[] but leaves
fb_display[i].mode set).
store_modes() relies on fb_new_modelist() -> fbcon_new_modelist() to move
the back-pointers to the new list, but that only re-points consoles still
mapped to the framebuffer. A console that is no longer mapped to it is
skipped, so once fb_destroy_modelist() frees the old list that console's
fb_display[i].mode dangles. A later FBIOPUT_VSCREENINFO with
FB_ACTIVATE_INV_MODE makes fbcon_mode_deleted() read it through
fb_mode_is_equal(), a use-after-free.
Commit a1f305893074 ("fbcon: Set fb_display[i]->mode to NULL when the
mode is released") fixed the same use-after-free for the framebuffer
unregister path. Do the same in store_modes() by clearing the stale
pointers with fbcon_delete_modelist() before freeing the old list.
Reported-by: syzbot+81c7c6b52649fd07299d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=81c7c6b52649fd07299d
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
This patch contains a proposed fix for a crash reported by syzbot in
fb_mode_is_equal().
The file names and offsets in this description are from commit
8cd9520d35a6c38db6567e97dd93b1f11f185dc6 on the for-next branch of
git://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git
I also have a deterministic reproducer that triggers the crash under virtme-ng
with a KASAN kernel and two framebuffers (vfb as fb0, bochs-drm as fb1). It
performs six ioctl/sysfs operations, listed at the end of "The Bug". The free
is driven by a write to /sys/class/graphics/fb0/modes, which needs write access
to that file. The reproducer was written with the help of a coding agent
(Claude Code).
This crash has the same KASAN signature as the one fixed by commit
a1f305893074 ("fbcon: Set fb_display[i]->mode to NULL when the mode is
released"), but reaches it through a different free path. It is still reported
on kernels that contain that commit.
The Bug
fb_display[i].mode is a back-pointer into a struct fb_modelist entry on
info->modelist. var_to_display() (fbcon.c:989) sets it when a console is
attached to a framebuffer. fbcon_mode_deleted() reads it through
fb_mode_is_equal() (fbcon.c:2750, modedb.c:931).
store_modes() is the .store handler for /sys/class/graphics/fbN/modes
(fbsysfs.c:94). It splices the current modelist into a local old_list, builds
the new list, and on success frees old_list with fb_destroy_modelist()
(fbsysfs.c:115). To keep the back-pointers valid it relies on
fb_new_modelist() -> fbcon_new_modelist() (fbmem.c:737), but that only re-points
consoles still mapped to this framebuffer (fbcon_info_from_console(i) != info,
fbcon.c:3038).
fb_display[i].mode is not cleared when a console is detached. Unbinding fbcon
runs fbcon_release_all() (fbcon.c:1248), which sets con2fb_map[i] = -1 but
leaves fb_display[i].mode pointing into the former framebuffer's modelist. Such
a console is skipped by fbcon_new_modelist(), so once fb_destroy_modelist()
frees old_list its fb_display[i].mode dangles.
The minimal reproduction is six operations on two framebuffers (fb0, fb1). All
six are required (verified against source and by removing each in turn). fbcon
starts bound with the consoles mapped to fb0. The operations all act on one
unused console i (no allocated vc_data). An allocated console would be
re-initialised, and its fb_display[i].mode reset, by the takeover in step 5.
Pre-allocating all consoles does not reproduce.
1. FBIOPUT_CON2FBMAP, the console -> fb1
set_con2fb_map() reaches var_to_display(), which points fb_display[i].mode
at an fb1 modelist entry (var_to_display() uses the console index, so it
runs even though the console has no vc_data)
2. FBIOPUT_CON2FBMAP, the console -> fb0
fb_display[i].mode now points at an fb0 modelist entry M
3. echo 0 > /sys/class/vtconsole/vtcon1/bind
fbcon_release_all() sets con2fb_map[i] = -1 and leaves fb_display[i].mode
pointing at M
4. write a videomode array to /sys/class/graphics/fb0/modes
The buffer holds raw struct fb_videomode entries. At least one must be
valid so fb_new_modelist() keeps it and store_modes() takes the success
path that frees old_list (an empty or invalid set takes the failure path
that restores it, and nothing is freed). fbcon_new_modelist() skips the
console (con2fb_map[i] == -1), then fb_destroy_modelist() frees M, so
fb_display[i].mode now dangles
5. FBIOPUT_CON2FBMAP, the console -> fb1
fbcon is unbound, so set_con2fb_map() takes the do_fbcon_takeover() branch
(fbcon.c:931). do_fbcon_takeover() sets con2fb_map[] = info_idx for the
whole range (fbcon.c:619), so the console is now mapped to fb1. The mode is
reset only via fbcon_init() during the takeover, which runs for allocated
consoles only, so this unused console keeps its dangling fb_display[i].mode
6. FBIOPUT_VSCREENINFO on fb1 with FB_ACTIVATE_INV_MODE and a mode different
from fb1's current var (otherwise fb_set_var() returns early without
calling fbcon_mode_deleted(), to avoid deleting the current var's
videomode)
fb_set_var() (fbmem.c:240,248) -> fbcon_mode_deleted() walks fb1's consoles
and reads the dangling fb_display[i].mode via fb_mode_is_equal(), a
use-after-free
The free is in store_modes() on fb0, and the read in fbcon_mode_deleted() on
fb1.
The Proposed Fix
A framebuffer's modelist is freed in two places, do_unregister_framebuffer()
(fbmem.c:548) and store_modes() (fbsysfs.c:115). Commit a1f305893074 added the
helper fbcon_delete_modelist() to clear the stale fb_display[i].mode pointers,
but called it from the first site only (fbmem.c:547). The store_modes() site
was raised as a potential issue during review of that commit [1] but
assumed safe.
Fix store_modes() the same way by clearing the stale pointers with
fbcon_delete_modelist() before freeing old_list.
The fix was verified with the reproducer. The unpatched kernel reports the
slab-use-after-free read, freed by fb_destroy_modelist() from store_modes().
The patched kernel produces no use-after-free.
The bug predates git history. Both the fb_display[i].mode back-pointer and the
store_modes() swap-and-free are present in the initial 2.6.12-rc2 import
(1da177e4c3f4), so there is no Fixes tag, matching a1f305893074. The fix calls
fbcon_delete_modelist(), which a1f305893074 added, so a stable backport needs
that commit first.
[1] https://lkml.org/lkml/2025/9/23/786
drivers/video/fbdev/core/fbsysfs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c
index baa2bae0fb5b..c761dcf21daf 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -111,8 +111,10 @@ static ssize_t store_modes(struct device *device,
if (fb_new_modelist(fb_info)) {
fb_destroy_modelist(&fb_info->modelist);
list_splice(&old_list, &fb_info->modelist);
- } else
+ } else {
+ fbcon_delete_modelist(&old_list);
fb_destroy_modelist(&old_list);
+ }
unlock_fb_info(fb_info);
console_unlock();
--
2.47.3
^ permalink raw reply related
* [PATCH] video: hpfb: Unregister DIO driver on init failure
From: Haoxiang Li @ 2026-06-22 6:49 UTC (permalink / raw)
To: deller; +Cc: linux-fbdev, dri-devel, linux-kernel, Haoxiang Li
hpfb_init() registers the DIO driver via dio_register_driver().
If a later error occurs, the function returns directly without
unregistering the DIO driver. Unregister the DIO driver before
returning from these error paths.
Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
---
drivers/video/fbdev/hpfb.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/hpfb.c b/drivers/video/fbdev/hpfb.c
index a1144b150982..1405712e5b4f 100644
--- a/drivers/video/fbdev/hpfb.c
+++ b/drivers/video/fbdev/hpfb.c
@@ -407,10 +407,13 @@ static int __init hpfb_init(void)
err = copy_from_kernel_nofault(&i, (unsigned char *)INTFBVADDR + DIO_IDOFF, 1);
if (!err && (i == DIO_ID_FBUFFER) && topcat_sid_ok(sid = DIO_SECID(INTFBVADDR))) {
- if (!request_mem_region(INTFBPADDR, DIO_DEVSIZE, "Internal Topcat"))
+ if (!request_mem_region(INTFBPADDR, DIO_DEVSIZE, "Internal Topcat")) {
+ dio_unregister_driver(&hpfb_driver);
return -EBUSY;
+ }
printk(KERN_INFO "Internal Topcat found (secondary id %02x)\n", sid);
if (hpfb_init_one(INTFBPADDR, INTFBVADDR)) {
+ dio_unregister_driver(&hpfb_driver);
return -ENOMEM;
}
}
--
2.25.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox