Re: [PATCH 2/2] mtd: Remove support for Carillo Ranch driver

2024-02-26 Thread Miquel Raynal
On Fri, 2023-12-08 at 22:47:03 UTC, "Matthew Wilcox (Oracle)" wrote:
> As far as anybody can tell, this product never shipped.  If it did,
> it shipped in 2007 and nobody has access to one any more.  Remove the
> mtd NOR driver.
> 
> Signed-off-by: Matthew Wilcox (Oracle) 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
mtd/next, thanks.

Miquel


Re: [PATCH 2/2] drm/vkms: Use a simpler composition function

2024-02-02 Thread Miquel Raynal
Hi Pekka,

pekka.paala...@haloniitty.fi wrote on Fri, 2 Feb 2024 17:49:13 +0200:

> On Fri, 2 Feb 2024 13:13:22 +0100
> Miquel Raynal  wrote:
> 
> > Hello Maxime,
> > 
> > + Arthur
> > 
> > mrip...@kernel.org wrote on Fri, 2 Feb 2024 10:53:37 +0100:
> >   
> > > Hi Miquel,
> > > 
> > > On Fri, Feb 02, 2024 at 10:26:01AM +0100, Miquel Raynal wrote:
> > > > pekka.paala...@haloniitty.fi wrote on Fri, 2 Feb 2024 10:55:22 +0200:
> > > >   
> > > > > On Thu, 01 Feb 2024 18:31:32 +0100
> > > > > Louis Chauvet  wrote:
> > > > >   
> > > > > > Change the composition algorithm to iterate over pixels instead of 
> > > > > > lines.
> > > > > > It allows a simpler management of rotation and pixel access for 
> > > > > > complex formats.
> > > > > > 
> > > > > > This new algorithm allows read_pixel function to have access to x/y
> > > > > > coordinates and make it possible to read the correct thing in a 
> > > > > > block
> > > > > > when block_w and block_h are not 1.
> > > > > > The iteration pixel-by-pixel in the same method also allows a 
> > > > > > simpler
> > > > > > management of rotation with drm_rect_* helpers. This way it's not 
> > > > > > needed
> > > > > > anymore to have misterious switch-case distributed in multiple 
> > > > > > places.
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > there was a very good reason to write this code using lines:
> > > > > performance. Before lines, it was indeed operating on individual 
> > > > > pixels.
> > > > > 
> > > > > Please, include performance measurements before and after this series
> > > > > to quantify the impact on the previously already supported pixel
> > > > > formats, particularly the 32-bit-per-pixel RGB variants.
> > > > > 
> > > > > VKMS will be used more and more in CI for userspace projects, and
> > > > > performance actually matters there.
> > > > > 
> > > > > I'm worrying that this performance degradation here is significant. I
> > > > > believe it is possible to keep blending with lines, if you add new 
> > > > > line
> > > > > getters for reading from rotated, sub-sampled etc. images. That way 
> > > > > you
> > > > > don't have to regress the most common formats' performance.  
> > > > 
> > > > While I understand performance is important and should be taken into
> > > > account seriously, I cannot understand how broken testing could be
> > > > considered better. Fast but inaccurate will always be significantly
> > > > less attractive to my eyes.  
> > > 
> > > AFAIK, neither the cover letter nor the commit log claimed it was fixing
> > > something broken, just that it was "better" (according to what
> > > criteria?).
> > 
> > Better is probably too vague and I agree the "fixing" part is not
> > clearly explained in the commit log. The cover-letter however states:
> >   
> > > Patch 2/2: This patch is more complex. My main target was to solve issues
> > > I found in [1], but as it was very complex to do it "in place", I choose
> > > to rework the composition function.
> > ...  
> > > [1]: 
> > > https://lore.kernel.org/dri-devel/20240110-vkms-yuv-v2-0-952fcaa5a...@riseup.net/
> > > 
> > 
> > If you follow this link you will find all the feedback and especially
> > the "broken" parts. Just to be clear, writing bugs is totally expected
> > and review/testing is supposed to help on this regard. I am not blaming
> > the author in any way, just focusing on getting this code in a more
> > readable shape and hopefully reinforce the testing procedure.
> >   
> > > If something is truly broken, it must be stated what exactly is so we
> > > can all come up with a solution that will satisfy everyone.
> > 
> > Maybe going through the series pointed above will give more context
> > but AFAIU: the YUV composition is not totally right (and the tests used
> > to validate it need to be more complex as well in order to fail).
> > 
> > Here is a proposal.
> > 
> > Today's RGB implementation is onl

Re: [PATCH 2/2] drm/vkms: Use a simpler composition function

2024-02-02 Thread Miquel Raynal
Hello Maxime,

+ Arthur

mrip...@kernel.org wrote on Fri, 2 Feb 2024 10:53:37 +0100:

> Hi Miquel,
> 
> On Fri, Feb 02, 2024 at 10:26:01AM +0100, Miquel Raynal wrote:
> > pekka.paala...@haloniitty.fi wrote on Fri, 2 Feb 2024 10:55:22 +0200:
> >   
> > > On Thu, 01 Feb 2024 18:31:32 +0100
> > > Louis Chauvet  wrote:
> > >   
> > > > Change the composition algorithm to iterate over pixels instead of 
> > > > lines.
> > > > It allows a simpler management of rotation and pixel access for complex 
> > > > formats.
> > > > 
> > > > This new algorithm allows read_pixel function to have access to x/y
> > > > coordinates and make it possible to read the correct thing in a block
> > > > when block_w and block_h are not 1.
> > > > The iteration pixel-by-pixel in the same method also allows a simpler
> > > > management of rotation with drm_rect_* helpers. This way it's not needed
> > > > anymore to have misterious switch-case distributed in multiple places.  
> > > >   
> > > 
> > > Hi,
> > > 
> > > there was a very good reason to write this code using lines:
> > > performance. Before lines, it was indeed operating on individual pixels.
> > > 
> > > Please, include performance measurements before and after this series
> > > to quantify the impact on the previously already supported pixel
> > > formats, particularly the 32-bit-per-pixel RGB variants.
> > > 
> > > VKMS will be used more and more in CI for userspace projects, and
> > > performance actually matters there.
> > > 
> > > I'm worrying that this performance degradation here is significant. I
> > > believe it is possible to keep blending with lines, if you add new line
> > > getters for reading from rotated, sub-sampled etc. images. That way you
> > > don't have to regress the most common formats' performance.  
> > 
> > While I understand performance is important and should be taken into
> > account seriously, I cannot understand how broken testing could be
> > considered better. Fast but inaccurate will always be significantly
> > less attractive to my eyes.  
> 
> AFAIK, neither the cover letter nor the commit log claimed it was fixing
> something broken, just that it was "better" (according to what
> criteria?).

Better is probably too vague and I agree the "fixing" part is not
clearly explained in the commit log. The cover-letter however states:

> Patch 2/2: This patch is more complex. My main target was to solve issues
> I found in [1], but as it was very complex to do it "in place", I choose
> to rework the composition function.
...
> [1]: 
> https://lore.kernel.org/dri-devel/20240110-vkms-yuv-v2-0-952fcaa5a...@riseup.net/

If you follow this link you will find all the feedback and especially
the "broken" parts. Just to be clear, writing bugs is totally expected
and review/testing is supposed to help on this regard. I am not blaming
the author in any way, just focusing on getting this code in a more
readable shape and hopefully reinforce the testing procedure.

> If something is truly broken, it must be stated what exactly is so we
> can all come up with a solution that will satisfy everyone.

Maybe going through the series pointed above will give more context
but AFAIU: the YUV composition is not totally right (and the tests used
to validate it need to be more complex as well in order to fail).

Here is a proposal.

Today's RGB implementation is only optimized in the line-by-line case
when there is no rotation. The logic is bit convoluted and may possibly
be slightly clarified with a per-format read_line() implementation,
at a very light performance cost. Such an improvement would definitely
benefit to the clarity of the code, especially when transformations
(especially the rotations) come into play because they would be clearly
handled differently instead of being "hidden" in the optimized logic.
Performances would not change much as this path is not optimized today
anyway (the pixel-oriented logic is already used in the rotation case).

Arthur's YUV implementation is indeed well optimized but the added
complexity probably lead to small mistakes in the logic. The
per-format read_line() implementation mentioned above could be
extended to the YUV format as well, which would leverage Arthur's
proposal by re-using his optimized version. Louis will help on this
regard. However, for more complex cases such as when there is a
rotation, it will be easier (and not sub-optimized compared to the RGB
case) to also fallback to a pixel-oriented processing.

Would this approach make sense?

Thanks,
Miquèl


Re: [PATCH 2/2] drm/vkms: Use a simpler composition function

2024-02-02 Thread Miquel Raynal
Hi Pekka,

pekka.paala...@haloniitty.fi wrote on Fri, 2 Feb 2024 10:55:22 +0200:

> On Thu, 01 Feb 2024 18:31:32 +0100
> Louis Chauvet  wrote:
> 
> > Change the composition algorithm to iterate over pixels instead of lines.
> > It allows a simpler management of rotation and pixel access for complex 
> > formats.
> > 
> > This new algorithm allows read_pixel function to have access to x/y
> > coordinates and make it possible to read the correct thing in a block
> > when block_w and block_h are not 1.
> > The iteration pixel-by-pixel in the same method also allows a simpler
> > management of rotation with drm_rect_* helpers. This way it's not needed
> > anymore to have misterious switch-case distributed in multiple places.  
> 
> Hi,
> 
> there was a very good reason to write this code using lines:
> performance. Before lines, it was indeed operating on individual pixels.
> 
> Please, include performance measurements before and after this series
> to quantify the impact on the previously already supported pixel
> formats, particularly the 32-bit-per-pixel RGB variants.
> 
> VKMS will be used more and more in CI for userspace projects, and
> performance actually matters there.
> 
> I'm worrying that this performance degradation here is significant. I
> believe it is possible to keep blending with lines, if you add new line
> getters for reading from rotated, sub-sampled etc. images. That way you
> don't have to regress the most common formats' performance.

While I understand performance is important and should be taken into
account seriously, I cannot understand how broken testing could be
considered better. Fast but inaccurate will always be significantly
less attractive to my eyes.

I am in favor of making this working first, and then improving the code
for faster results. Maybe the line-driven approach can be dedicated to
"simpler" formats where more complex corner cases do not happen. But
for now I don't see the point in comparing performances between broken
and (hopefully) non broken implementations.

Thanks,
Miquèl


Re: [RFC PATCH] drm/imx: ipuv3-plane: Allow preventing sequential DMA bursts

2023-11-17 Thread Miquel Raynal
Hello,

miquel.ray...@bootlin.com wrote on Fri, 27 Oct 2023 18:20:25 +0200:

> Sequential DMA bursts improve NIC/RAM usage thanks to the basic NIC
> hardware optimizations available when performing in-order sequential
> accesses. This can be further enforced with the IPU DMA locking
> mechanism which basically prevents any other IP to access the
> interconnect for a longer time while performing up to 8 sequential DMA
> bursts. The drawback is a lower availability for short time periods and
> delayed accesses which may cause problem with latency-sensible systems
> (typically, the network might suffer from high drop rates). This is even
> more visible with larger displays requiring even more RAM bandwidth.
> 
> Issues have been observed on IMX6Q. The setup featured a 60Hz 1024x768
> LVDS display just showing a static picture (thus no CPU usage, only
> background DMA bringing the picture to the display engine). When
> performing full speed iperf3 uplink tests with the FEC, almost no drop
> was observed, whereas the drop would raise above 50% when limiting the
> bandwidth to 1Mb/s (on a 100Mb/s link). The exact same test with the
> display pipeline disabled would show little to no drop. The LP-DDR3 chip
> on the module would allow up to ~53MiB each 1/60th of a second, and the
> display pipeline consume approximately ~10MiB of this bandwidth, and
> thus be active 20% of the time on each time slot.
> 
> One particular feature of the IPU DMA controller (IDMAC) is the ability
> to serialize DMA bursts and to lock further interconnect accesses when
> doing so. Experimentally, disabling the locking lead to a drop rate from
> 50% down to 10%. A few more % could be earned by setting the burst
> number to 1. It seems this huge difference could be explained by a
> possible hardware conflict between the locking feature and some QoS
> logic. Indeed, on IMX6Q, the NIC-301 manages priorities and by default
> will elect ENET's requests (priority 2) above IPU's requests (priority
> 0). But the QoS seems to only be valid above a certain threshold, which
> is: 4 consequent DMA bursts in the case of the IPU. It was indeed
> observed that tweaking the number of bursts to be lowered from 8 to 4
> would lead to a significant increase in the Ethernet transfers
> stability. IOW, it looks like when the display pipeline performs DMA
> transfers, incoming DMA requests from other master devices on the
> interconnect are delayed too much (or canceled).
> 
> I have no clue to explain why on the Ethernet MAC side some uDMA
> transfers would never reach completion, especially without notification
> nor any error. All uplink transfers are properly queued at the FEC level
> and more importantly, the corresponding interrupts are fired upon
> "proper transmission" and report no error whatsoever (note: there is no
> actual way to know the uDMA internal controller could not fetch the
> data, only MAC errors could be reported at this stage).
> 
> As a solution, we might want to prevent these DMA bursts from being
> queued together. Maybe the IMX6Q is primarily used for its graphics
> capabilities, but when the network (and other RAM consuming subsystem)
> also matter, it may be relevant to apply this workaround in order to
> help them fetching from RAM more reliably.
> 
> Signed-off-by: Miquel Raynal 
> ---
> 
> Hello,
> 
> This really is an RFC as the bug was also observed on v6.5 but the fix
> proposed here was written and tested on a v4.14 kernel. I want to
> discuss the approach and ideally get some feedback from imx6 experts who
> know the SoC internals before publishing a clean series. There is a lot
> of guessing in this workaround, besides the experimental measures I
> managed to do. I would be glad if someone could sched any light or
> involve knowledgeable people in this conversation.
> 
> The initial report was there and mainly focused on the network
> subsystem:
> https://lore.kernel.org/netdev/18b72fdb-d24a-a416-ffab-3a15b281a...@katalix.com/T/#md265d6da81b8fb6b85e3adbb399bcda79dfc761c
> In this thread I made wrong observations because for speeding up my test
> cycles, I dropped the support for: DRM, SND, USB as these subsystems
> seemed totally irrelevant. It actually had a strong impact.
> 
> In the end, I really think there is something wrong with the locking of
> IPU DMA bursts when mixed with the QoS of the NIC.

Further investigation lead to the DDR configuration itself. The
system worked perfectly besides the Ethernet drop rate which was
abnormally high and it turns out, just changing a bit in the DDR reset
pad configuration fixed it. I cannot explain exactly what was the root
cause but it is possible that the DDR was in a relatively unstable
state due to the power-on/reset procedure not being followed correctly
due to the incomplete pad configuration.

Here is the U-Boot thread I've started: 
https://lore.kernel.org/u-boot/20231117150044.1792080-1-miquel.ray...@bootlin.com/

Thanks,
Miquèl


[RFC PATCH] drm/imx: ipuv3-plane: Allow preventing sequential DMA bursts

2023-10-27 Thread Miquel Raynal
Sequential DMA bursts improve NIC/RAM usage thanks to the basic NIC
hardware optimizations available when performing in-order sequential
accesses. This can be further enforced with the IPU DMA locking
mechanism which basically prevents any other IP to access the
interconnect for a longer time while performing up to 8 sequential DMA
bursts. The drawback is a lower availability for short time periods and
delayed accesses which may cause problem with latency-sensible systems
(typically, the network might suffer from high drop rates). This is even
more visible with larger displays requiring even more RAM bandwidth.

Issues have been observed on IMX6Q. The setup featured a 60Hz 1024x768
LVDS display just showing a static picture (thus no CPU usage, only
background DMA bringing the picture to the display engine). When
performing full speed iperf3 uplink tests with the FEC, almost no drop
was observed, whereas the drop would raise above 50% when limiting the
bandwidth to 1Mb/s (on a 100Mb/s link). The exact same test with the
display pipeline disabled would show little to no drop. The LP-DDR3 chip
on the module would allow up to ~53MiB each 1/60th of a second, and the
display pipeline consume approximately ~10MiB of this bandwidth, and
thus be active 20% of the time on each time slot.

One particular feature of the IPU DMA controller (IDMAC) is the ability
to serialize DMA bursts and to lock further interconnect accesses when
doing so. Experimentally, disabling the locking lead to a drop rate from
50% down to 10%. A few more % could be earned by setting the burst
number to 1. It seems this huge difference could be explained by a
possible hardware conflict between the locking feature and some QoS
logic. Indeed, on IMX6Q, the NIC-301 manages priorities and by default
will elect ENET's requests (priority 2) above IPU's requests (priority
0). But the QoS seems to only be valid above a certain threshold, which
is: 4 consequent DMA bursts in the case of the IPU. It was indeed
observed that tweaking the number of bursts to be lowered from 8 to 4
would lead to a significant increase in the Ethernet transfers
stability. IOW, it looks like when the display pipeline performs DMA
transfers, incoming DMA requests from other master devices on the
interconnect are delayed too much (or canceled).

I have no clue to explain why on the Ethernet MAC side some uDMA
transfers would never reach completion, especially without notification
nor any error. All uplink transfers are properly queued at the FEC level
and more importantly, the corresponding interrupts are fired upon
"proper transmission" and report no error whatsoever (note: there is no
actual way to know the uDMA internal controller could not fetch the
data, only MAC errors could be reported at this stage).

As a solution, we might want to prevent these DMA bursts from being
queued together. Maybe the IMX6Q is primarily used for its graphics
capabilities, but when the network (and other RAM consuming subsystem)
also matter, it may be relevant to apply this workaround in order to
help them fetching from RAM more reliably.

Signed-off-by: Miquel Raynal 
---

Hello,

This really is an RFC as the bug was also observed on v6.5 but the fix
proposed here was written and tested on a v4.14 kernel. I want to
discuss the approach and ideally get some feedback from imx6 experts who
know the SoC internals before publishing a clean series. There is a lot
of guessing in this workaround, besides the experimental measures I
managed to do. I would be glad if someone could sched any light or
involve knowledgeable people in this conversation.

The initial report was there and mainly focused on the network
subsystem:
https://lore.kernel.org/netdev/18b72fdb-d24a-a416-ffab-3a15b281a...@katalix.com/T/#md265d6da81b8fb6b85e3adbb399bcda79dfc761c
In this thread I made wrong observations because for speeding up my test
cycles, I dropped the support for: DRM, SND, USB as these subsystems
seemed totally irrelevant. It actually had a strong impact.

In the end, I really think there is something wrong with the locking of
IPU DMA bursts when mixed with the QoS of the NIC.

Any comments welcome!
[I know this deserves a DT binding entry, but we are not there yet]

Thanks,
Miquèl

 drivers/gpu/drm/imx/ipuv3-plane.c | 27 ---
 drivers/gpu/drm/imx/ipuv3-plane.h |  1 +
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c 
b/drivers/gpu/drm/imx/ipuv3-plane.c
index cf98596c7ce1..0492df96ac3e 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -496,8 +496,8 @@ static int ipu_chan_assign_axi_id(int ipu_chan)
}
 }
 
-static void ipu_calculate_bursts(u32 width, u32 cpp, u32 stride,
-u8 *burstsize, u8 *num_bursts)
+static void ipu_calculate_bursts(struct ipu_plane *ipu_plane, u32 width, u32 
cpp,
+u32 stride, u8 *burstsize, u8 *

Re: [PATCH] drm: atmel-hlcdc: Support inverting the pixel clock polarity

2023-08-10 Thread Miquel Raynal
Hi Boris,

s...@ravnborg.org wrote on Thu, 10 Aug 2023 19:31:25 +0200:

> > I queued it to drm-misc-next this morning.  

Yeah, thanks a lot!

Cheers,
Miquèl


Re: [PATCH] drm: atmel-hlcdc: Support inverting the pixel clock polarity

2023-08-08 Thread Miquel Raynal
Hi Sam,

s...@ravnborg.org wrote on Mon, 7 Aug 2023 18:52:45 +0200:

> Hi Miquel,
> 
> On Mon, Aug 07, 2023 at 11:12:46AM +0200, Miquel Raynal wrote:
> > Hi Sam,
> > 
> > s...@ravnborg.org wrote on Sat, 10 Jun 2023 22:05:15 +0200:
> >   
> > > On Fri, Jun 09, 2023 at 04:48:43PM +0200, Miquel Raynal wrote:  
> > > > On the SoC host controller, the pixel clock can be:
> > > > * standard: data is launched on the rising edge
> > > > * inverted: data is launched on the falling edge
> > > > 
> > > > Some panels may need the inverted option to be used so let's support
> > > > this DRM flag.
> > > > 
> > > > Signed-off-by: Miquel Raynal 
> > > 
> > > Hi Miquel,
> > > 
> > > the patch is:
> > > Reviewed-by: Sam Ravnborg 
> > > 
> > > I hope someone else can pick it up and apply it to drm-misc as
> > > my drm-misc setup is hopelessly outdated atm.  
> > 
> > I haven't been noticed this patch was picked-up, is your tree still
> > outdated or can you take care of it?  
> 
> I am still hopelessly behind on stuff.

No problem.

> I copied a few people on this mail that I hope can help.

Nice, thanks a lot!

> Link to the original patch:
> https://lore.kernel.org/dri-devel/20230609144843.851327-1-miquel.ray...@bootlin.com/
> 
>   Sam

Let me know in case it's easier if I re-send it.

Thanks,
Miquèl


[PATCH v4 1/2] dt-bindings: display: simple: Add Mitsubishi AA084XE01 panel

2023-08-07 Thread Miquel Raynal
Add Mitsubishi AA084XE01 8.4" XGA TFT LCD panel compatible string.

Link: https://www.mouser.fr/datasheet/2/274/aa084xe01_e-364171.pdf
Signed-off-by: Miquel Raynal 
Acked-by: Krzysztof Kozlowski 
---

Changes in v4:
* Add datasheet link.
* Supplement Cc: list.

Changes in v3:
* None.

Changes in v2:
* Collected tag.

 .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml 
b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 1d4936fc5182..2c6be20bafc9 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -240,6 +240,8 @@ properties:
   - logictechno,lttd800480070-l6wh-rt
 # Mitsubishi "AA070MC01 7.0" WVGA TFT LCD panel
   - mitsubishi,aa070mc01-ca1
+# Mitsubishi AA084XE01 8.4" XGA TFT LCD panel
+  - mitsubishi,aa084xe01
 # Multi-Inno Technology Co.,Ltd MI0700S4T-6 7" 800x480 TFT Resistive 
Touch Module
   - multi-inno,mi0700s4t-6
 # Multi-Inno Technology Co.,Ltd MI0800FT-9 8" 800x600 TFT Resistive 
Touch Module
-- 
2.34.1



[PATCH v4 2/2] drm/panel: simple: Add support for Mitsubishi AA084XE01

2023-08-07 Thread Miquel Raynal
From: Thomas Weber 

Add support for the Mitsubishi AA084XE01 panel which is an 8.4 inch XGA
TFT-LCD module for industrial use.

Link: https://www.mouser.fr/datasheet/2/274/aa084xe01_e-364171.pdf
Signed-off-by: Thomas Weber 
Signed-off-by: Miquel Raynal 
---

Changes in v4:
* None.

Changes in v3:
* Fix connector type.

Changes in v2:
* Add connector type and bus flags.

 drivers/gpu/drm/panel/panel-simple.c | 29 
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index a247a0e7c799..e498a40e1f78 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2763,6 +2763,32 @@ static const struct panel_desc mitsubishi_aa070mc01 = {
.bus_flags = DRM_BUS_FLAG_DE_HIGH,
 };
 
+static const struct drm_display_mode mitsubishi_aa084xe01_mode = {
+   .clock = 56234,
+   .hdisplay = 1024,
+   .hsync_start = 1024 + 24,
+   .hsync_end = 1024 + 24 + 63,
+   .htotal = 1024 + 24 + 63 + 1,
+   .vdisplay = 768,
+   .vsync_start = 768 + 3,
+   .vsync_end = 768 + 3 + 6,
+   .vtotal = 768 + 3 + 6 + 1,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
+};
+
+static const struct panel_desc mitsubishi_aa084xe01 = {
+   .modes = _aa084xe01_mode,
+   .num_modes = 1,
+   .bpc = 8,
+   .size = {
+   .width = 1024,
+   .height = 768,
+   },
+   .bus_format = MEDIA_BUS_FMT_RGB565_1X16,
+   .connector_type = DRM_MODE_CONNECTOR_DPI,
+   .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE,
+};
+
 static const struct display_timing multi_inno_mi0700s4t_6_timing = {
.pixelclock = { 2900, 3300, 3800 },
.hactive = { 800, 800, 800 },
@@ -4286,6 +4312,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "mitsubishi,aa070mc01-ca1",
.data = _aa070mc01,
+   }, {
+   .compatible = "mitsubishi,aa084xe01",
+   .data = _aa084xe01,
}, {
.compatible = "multi-inno,mi0700s4t-6",
.data = _inno_mi0700s4t_6,
-- 
2.34.1



Re: [PATCH] drm: atmel-hlcdc: Support inverting the pixel clock polarity

2023-08-07 Thread Miquel Raynal
Hi Sam,

s...@ravnborg.org wrote on Sat, 10 Jun 2023 22:05:15 +0200:

> On Fri, Jun 09, 2023 at 04:48:43PM +0200, Miquel Raynal wrote:
> > On the SoC host controller, the pixel clock can be:
> > * standard: data is launched on the rising edge
> > * inverted: data is launched on the falling edge
> > 
> > Some panels may need the inverted option to be used so let's support
> > this DRM flag.
> > 
> > Signed-off-by: Miquel Raynal   
> 
> Hi Miquel,
> 
> the patch is:
> Reviewed-by: Sam Ravnborg 
> 
> I hope someone else can pick it up and apply it to drm-misc as
> my drm-misc setup is hopelessly outdated atm.

I haven't been noticed this patch was picked-up, is your tree still
outdated or can you take care of it?

Thanks a lot,
Miquèl


Re: [PATCH v3 00/19] Sitronix ST7789V improvements

2023-07-15 Thread Miquel Raynal
Hi Sebastian,

+ Thomas

s...@kernel.org wrote on Fri, 14 Jul 2023 03:37:37 +0200:

> Hi,
> 
> This adds panel support for Inanbo T28CP45TN89, which I found inside of a
> handheld thermal camera. The panel is based on the st7789v controller. All
> information is based on reverse engineering. I also appended the series
> from Miquel Raynal adding EDT ET028013DMA panel support, so that I could
> easily test it with my SPI_NO_RX setup. They are slightly different due
> to rebasing.

Thanks a lot! I'll continue following the series and provide my help
when required.

Cheers,
Miquèl

> 
> Changes since PATCHv2:
>  * https://lore.kernel.org/all/20230422205012.2464933-1-...@kernel.org/
>  * 
> https://lore.kernel.org/all/20230616163255.2804163-1-miquel.ray...@bootlin.com/
>  * Add Rob Herring's R-b for the DT binding
>  * Make panel info "static const"
>  * Add Michael Riesch's R-b to all my patches
>  * Rebase to 6.5-rc1
>  * Append Miquel's series
> 
> Changes since PATCHv1:
>  * https://lore.kernel.org/all/20230317232355.1554980-1-...@kernel.org/
>  * Apply DT binding changes requested by Krzysztof Kozlowski and his Ack
>  * I changed the driver patches to avoid code duplication and splitted
>the code a bit more
> 
> Greetings,
> 
> -- Sebastian
> 
> Miquel Raynal (6):
>   dt-bindings: display: st7789v: Add the edt,et028013dma panel
> compatible
>   dt-bindings: display: st7789v: bound the number of Rx data lines
>   drm/panel: sitronix-st7789v: Use 9 bits per spi word by default
>   drm/panel: sitronix-st7789v: Clarify a definition
>   drm/panel: sitronix-st7789v: Add EDT ET028013DMA panel support
>   drm/panel: sitronix-st7789v: Check display ID
> 
> Sebastian Reichel (13):
>   dt-bindings: vendor-prefixes: add Inanbo
>   dt-bindings: display: st7789v: add Inanbo T28CP45TN89
>   drm/panel: sitronix-st7789v: add SPI ID table
>   drm/panel: sitronix-st7789v: remove unused constants
>   drm/panel: sitronix-st7789v: make reset GPIO optional
>   drm/panel: sitronix-st7789v: simplify st7789v_spi_write
>   drm/panel: sitronix-st7789v: improve error handling
>   drm/panel: sitronix-st7789v: avoid hardcoding mode info
>   drm/panel: sitronix-st7789v: avoid hardcoding panel size
>   drm/panel: sitronix-st7789v: add media bus format
>   drm/panel: sitronix-st7789v: avoid hardcoding invert mode
>   drm/panel: sitronix-st7789v: avoid hardcoding polarity info
>   drm/panel: sitronix-st7789v: add Inanbo T28CP45TN89 support
> 
>  .../display/panel/sitronix,st7789v.yaml   |  10 +-
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  .../gpu/drm/panel/panel-sitronix-st7789v.c| 262 +++---
>  3 files changed, 237 insertions(+), 37 deletions(-)
> 


Re: [PATCH v2 00/13] Add Inanbo T28CP45TN89 panel support

2023-07-12 Thread Miquel Raynal
Hi Sebastian,

+ Thomas

s...@kernel.org wrote on Sat, 22 Apr 2023 22:49:59 +0200:

> Hi,
> 
> This adds panel support for Inanbo T28CP45TN89, which I found inside of a
> handheld thermal camera. The panel is based on the st7789v controller. All
> information is based on reverse engineering.

I haven't seen another version for this series so I assume it is still
the one to look at. As you already know, I also want to add support for
a panel based on the st7789 display controller. As discussed, I rebased
my changes on top of yours as you actually sent them upstream
much earlier than I did.

As a single minor comment was made to this version of the series, I
would like to know if you wanted to send a new version soon? Or if it
would make sense to send a bigger series with all our common patches in
one single shot (mine should apply cleanly without further work on
yours). Let me know how we can move forward.

For the record, here are my patches.

Link: 
https://lore.kernel.org/all/20230619155958.3119181-1-miquel.ray...@bootlin.com/

Thanks a lot,
Miquèl

> 
> Changes since PATCHv1:
>  * https://lore.kernel.org/all/20230317232355.1554980-1-...@kernel.org/
>  * Apply DT binding changes requested by Krzysztof Kozlowski and his Ack
>  * I changed the driver patches to avoid code duplication and splitted
>the code a bit more
> 
> -- Sebastian
> 
> Sebastian Reichel (13):
>   dt-bindings: vendor-prefixes: add Inanbo
>   dt-bindings: display: st7789v: add Inanbo T28CP45TN89
>   drm/panel: sitronix-st7789v: add SPI ID table
>   drm/panel: sitronix-st7789v: remove unused constants
>   drm/panel: sitronix-st7789v: make reset GPIO optional
>   drm/panel: sitronix-st7789v: simplify st7789v_spi_write
>   drm/panel: sitronix-st7789v: improve error handling
>   drm/panel: sitronix-st7789v: avoid hardcoding mode info
>   drm/panel: sitronix-st7789v: avoid hardcoding panel size
>   drm/panel: sitronix-st7789v: add media bus format
>   drm/panel: sitronix-st7789v: avoid hardcoding invert mode
>   drm/panel: sitronix-st7789v: avoid hardcoding polarity info
>   drm/panel: sitronix-st7789v: add Inanbo T28CP45TN89 support
> 
>  .../display/panel/sitronix,st7789v.yaml   |   5 +-
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  .../gpu/drm/panel/panel-sitronix-st7789v.c| 148 ++
>  3 files changed, 120 insertions(+), 35 deletions(-)
> 



Re: [PATCH v3 0/2] Small of/device cleanup

2023-07-12 Thread Miquel Raynal
Hello,

miquel.ray...@bootlin.com wrote on Thu, 22 Jun 2023 23:32:12 +0200:

> My previous attempt to slightly clean the OF core wrt device structures
> was rather unsuccessful as the idea behind the discussed cleanup was
> more impacting than what I thought, leading to most of the previous
> series to be dropped. However, aside, two patches seemed actually
> relevant, so here they are, alone.
> 
> Link: https://lore.kernel.org/all/20230608184903.ga3200973-r...@kernel.org/

I expect this small series to go through the drm tree, but as I actually
sent it right before the beginning of the merge window and because I am
not an experienced drm contributor, I would like to know if I am
required to resend the patches or if they are fine as-is (I don't
expect any conflicts with v6.5-rc1).

Just let me know if a re-send is expected.

Cheers,
Miquèl

> 
> Thanks,
> Miquèl
> 
> Changes in v3:
> * Fixed the dev->parent referencing in the host1x driver.
> * Collected Rob's Acked-by.
> 
> Changes in v2:
> * Dropped all the of_device.h/of_module.h changes
> * Directly used of_device_uevent() from the host1x driver
> 
> 
> Miquel Raynal (2):
>   of: module: Export of_device_uevent()
>   gpu: host1x: Stop open-coding of_device_uevent()
> 
>  drivers/gpu/host1x/bus.c | 29 ++---
>  drivers/of/device.c  |  1 +
>  2 files changed, 7 insertions(+), 23 deletions(-)
> 



Re: [PATCH] drm: atmel-hlcdc: Support inverting the pixel clock polarity

2023-07-12 Thread Miquel Raynal
Hello,

s...@ravnborg.org wrote on Sat, 10 Jun 2023 22:05:15 +0200:

> On Fri, Jun 09, 2023 at 04:48:43PM +0200, Miquel Raynal wrote:
> > On the SoC host controller, the pixel clock can be:
> > * standard: data is launched on the rising edge
> > * inverted: data is launched on the falling edge
> > 
> > Some panels may need the inverted option to be used so let's support
> > this DRM flag.
> > 
> > Signed-off-by: Miquel Raynal   
> 
> Hi Miquel,
> 
> the patch is:
> Reviewed-by: Sam Ravnborg 
> 
> I hope someone else can pick it up and apply it to drm-misc as
> my drm-misc setup is hopelessly outdated atm.

Looks like nobody picked this up yet, can someone take it? Let me know
if you want me to send it again.

Thanks,
Miquèl


[PATCH v3 2/2] gpu: host1x: Stop open-coding of_device_uevent()

2023-06-22 Thread Miquel Raynal
There is apparently no reasons to open-code of_device_uevent() besides:
- The helper receives a struct device while we want to use the of_node
  member of the struct device *parent*.
- of_device_uevent() could not be called by modules because of a missing
  EXPORT_SYMBOL*().

In practice, the former point is not very constraining, just calling
of_device_uevent(dev->parent, ...) would have made the trick.

The latter point is more an observation rather than a real blocking
point because nothing prevented of_uevent() (called by the inline
function of_device_uevent()) to be exported to modules. In practice,
this helper is now exported, so nothing prevent us from using
of_device_uevent() anymore.

Let's use the core helper directly instead of open-coding it.

Cc: Thierry Reding 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Mikko Perttunen 
Cc: Rob Herring 
Cc: Frank Rowand 
Signed-off-by: Miquel Raynal 
---
 drivers/gpu/host1x/bus.c | 29 ++---
 1 file changed, 6 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
index 4d16a3396c4a..84d042796d2e 100644
--- a/drivers/gpu/host1x/bus.c
+++ b/drivers/gpu/host1x/bus.c
@@ -338,32 +338,15 @@ static int host1x_device_match(struct device *dev, struct 
device_driver *drv)
return strcmp(dev_name(dev), drv->name) == 0;
 }
 
+/*
+ * Note that this is really only needed for backwards compatibility
+ * with libdrm, which parses this information from sysfs and will
+ * fail if it can't find the OF_FULLNAME, specifically.
+ */
 static int host1x_device_uevent(const struct device *dev,
struct kobj_uevent_env *env)
 {
-   struct device_node *np = dev->parent->of_node;
-   unsigned int count = 0;
-   struct property *p;
-   const char *compat;
-
-   /*
-* This duplicates most of of_device_uevent(), but the latter cannot
-* be called from modules and operates on dev->of_node, which is not
-* available in this case.
-*
-* Note that this is really only needed for backwards compatibility
-* with libdrm, which parses this information from sysfs and will
-* fail if it can't find the OF_FULLNAME, specifically.
-*/
-   add_uevent_var(env, "OF_NAME=%pOFn", np);
-   add_uevent_var(env, "OF_FULLNAME=%pOF", np);
-
-   of_property_for_each_string(np, "compatible", p, compat) {
-   add_uevent_var(env, "OF_COMPATIBLE_%u=%s", count, compat);
-   count++;
-   }
-
-   add_uevent_var(env, "OF_COMPATIBLE_N=%u", count);
+   of_device_uevent(dev->parent, env);
 
return 0;
 }
-- 
2.34.1



[PATCH v3 0/2] Small of/device cleanup

2023-06-22 Thread Miquel Raynal
My previous attempt to slightly clean the OF core wrt device structures
was rather unsuccessful as the idea behind the discussed cleanup was
more impacting than what I thought, leading to most of the previous
series to be dropped. However, aside, two patches seemed actually
relevant, so here they are, alone.

Link: https://lore.kernel.org/all/20230608184903.ga3200973-r...@kernel.org/

Thanks,
Miquèl

Changes in v3:
* Fixed the dev->parent referencing in the host1x driver.
* Collected Rob's Acked-by.

Changes in v2:
* Dropped all the of_device.h/of_module.h changes
* Directly used of_device_uevent() from the host1x driver


Miquel Raynal (2):
  of: module: Export of_device_uevent()
  gpu: host1x: Stop open-coding of_device_uevent()

 drivers/gpu/host1x/bus.c | 29 ++---
 drivers/of/device.c  |  1 +
 2 files changed, 7 insertions(+), 23 deletions(-)

-- 
2.34.1



[PATCH v3 1/2] of: module: Export of_device_uevent()

2023-06-22 Thread Miquel Raynal
The content of of_device_uevent() is currently hardcoded in a driver
that can be compiled as a module. Nothing prevents of_device_uevent() to
be exported to modules, most of the other helpers in of/device.c
actually are. The reason why this helper was not exported is because it
has been so far only useful in drivers/base, which is built-in anyway.

With the idea of getting rid of the hardcoded implementation of
of_device_uevent() in other places in the kernel, let's export it to GPL
modules (very much like its cousins in the same file).

Signed-off-by: Miquel Raynal 
Acked-by: Rob Herring 
---
 drivers/of/device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 0f00f1b80708..90131de6d75b 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -312,6 +312,7 @@ void of_device_uevent(const struct device *dev, struct 
kobj_uevent_env *env)
}
mutex_unlock(_mutex);
 }
+EXPORT_SYMBOL_GPL(of_device_uevent);
 
 int of_device_uevent_modalias(const struct device *dev, struct kobj_uevent_env 
*env)
 {
-- 
2.34.1



Re: [PATCH v2 2/2] gpu: host1x: Stop open-coding of_device_uevent()

2023-06-22 Thread Miquel Raynal
Hi Rob,

r...@kernel.org wrote on Thu, 22 Jun 2023 08:31:40 -0600:

> On Fri, Jun 09, 2023 at 05:56:34PM +0200, Miquel Raynal wrote:
> > There is apparently no reasons to open-code of_device_uevent() besides:
> > - The helper receives a struct device while we want to use the of_node
> >   member of the struct device *parent*.
> > - of_device_uevent() could not be called by modules because of a missing
> >   EXPORT_SYMBOL*().
> > 
> > In practice, the former point is not very constraining, just calling
> > of_device_uevent(dev->parent, ...) would have made the trick.
> > 
> > The latter point is more an observation rather than a real blocking
> > point because nothing prevented of_uevent() (called by the inline
> > function of_device_uevent()) to be exported to modules. In practice,
> > this helper is now exported, so nothing prevent us from using
> > of_device_uevent() anymore.
> > 
> > Let's use the core helper directly instead of open-coding it.
> > 
> > Cc: Thierry Reding 
> > Cc: David Airlie 
> > Cc: Daniel Vetter 
> > Cc: Mikko Perttunen 
> > Cc: Rob Herring 
> > Cc: Frank Rowand 
> > Signed-off-by: Miquel Raynal 
> > 
> > ---
> > 
> > This patch depends on the changes performed earlier in the series under
> > the drivers/of/ folder.
> > ---
> >  drivers/gpu/host1x/bus.c | 29 ++---
> >  1 file changed, 6 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
> > index 4d16a3396c4a..dae589b83be1 100644
> > --- a/drivers/gpu/host1x/bus.c
> > +++ b/drivers/gpu/host1x/bus.c
> > @@ -338,32 +338,15 @@ static int host1x_device_match(struct device *dev, 
> > struct device_driver *drv)
> > return strcmp(dev_name(dev), drv->name) == 0;
> >  }
> >  
> > +/*
> > + * Note that this is really only needed for backwards compatibility
> > + * with libdrm, which parses this information from sysfs and will
> > + * fail if it can't find the OF_FULLNAME, specifically.
> > + */
> >  static int host1x_device_uevent(const struct device *dev,
> > struct kobj_uevent_env *env)
> >  {
> > -   struct device_node *np = dev->parent->of_node;
> > -   unsigned int count = 0;
> > -   struct property *p;
> > -   const char *compat;
> > -
> > -   /*
> > -* This duplicates most of of_device_uevent(), but the latter cannot
> > -* be called from modules and operates on dev->of_node, which is not
> > -* available in this case.
> > -*
> > -* Note that this is really only needed for backwards compatibility
> > -* with libdrm, which parses this information from sysfs and will
> > -* fail if it can't find the OF_FULLNAME, specifically.
> > -*/
> > -   add_uevent_var(env, "OF_NAME=%pOFn", np);
> > -   add_uevent_var(env, "OF_FULLNAME=%pOF", np);
> > -
> > -   of_property_for_each_string(np, "compatible", p, compat) {
> > -   add_uevent_var(env, "OF_COMPATIBLE_%u=%s", count, compat);
> > -   count++;
> > -   }
> > -
> > -   add_uevent_var(env, "OF_COMPATIBLE_N=%u", count);
> > +   of_device_uevent((const struct device *)>parent, env);  
> 
> Why do you have the cast and the "&"?

Actually that's a mistake, I was blurred by the "I want a const pointer
and this is not a const pointer" warning (hence the cast). This change
is broken, I'll fix it.

Thanks,
Miquèl


[PATCH v3 1/2] dt-bindings: display: simple: Add Mitsubishi AA084XE01 panel

2023-06-22 Thread Miquel Raynal
Add Mitsubishi AA084XE01 8.4" XGA TFT LCD panel compatible string.

Signed-off-by: Miquel Raynal 
Acked-by: Krzysztof Kozlowski 
---

Changes in v3:
* None.

Changes in v2:
* Collected tag.

 .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml 
b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 18241f4051d2..cc841cf96fae 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -232,6 +232,8 @@ properties:
   - logictechno,lttd800480070-l6wh-rt
 # Mitsubishi "AA070MC01 7.0" WVGA TFT LCD panel
   - mitsubishi,aa070mc01-ca1
+# Mitsubishi AA084XE01 8.4" XGA TFT LCD panel
+  - mitsubishi,aa084xe01
 # Multi-Inno Technology Co.,Ltd MI0700S4T-6 7" 800x480 TFT Resistive 
Touch Module
   - multi-inno,mi0700s4t-6
 # Multi-Inno Technology Co.,Ltd MI0800FT-9 8" 800x600 TFT Resistive 
Touch Module
-- 
2.34.1



[PATCH v3 2/2] drm/panel: simple: Add support for Mitsubishi AA084XE01

2023-06-22 Thread Miquel Raynal
From: Thomas Weber 

Add support for the Mitsubishi AA084XE01 panel which is an 8.4 inch XGA
TFT-LCD module for industrial use.

Link: https://www.mouser.fr/datasheet/2/274/aa084xe01_e-364171.pdf
Signed-off-by: Thomas Weber 
Signed-off-by: Miquel Raynal 
---

Changes in v3:
* Fix connector type.

Changes in v2:
* Add connector type and bus flags.

 drivers/gpu/drm/panel/panel-simple.c | 29 
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 8a3b685c2fcc..cd2f8025aee0 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2670,6 +2670,32 @@ static const struct panel_desc mitsubishi_aa070mc01 = {
.bus_flags = DRM_BUS_FLAG_DE_HIGH,
 };
 
+static const struct drm_display_mode mitsubishi_aa084xe01_mode = {
+   .clock = 56234,
+   .hdisplay = 1024,
+   .hsync_start = 1024 + 24,
+   .hsync_end = 1024 + 24 + 63,
+   .htotal = 1024 + 24 + 63 + 1,
+   .vdisplay = 768,
+   .vsync_start = 768 + 3,
+   .vsync_end = 768 + 3 + 6,
+   .vtotal = 768 + 3 + 6 + 1,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
+};
+
+static const struct panel_desc mitsubishi_aa084xe01 = {
+   .modes = _aa084xe01_mode,
+   .num_modes = 1,
+   .bpc = 8,
+   .size = {
+   .width = 1024,
+   .height = 768,
+   },
+   .bus_format = MEDIA_BUS_FMT_RGB565_1X16,
+   .connector_type = DRM_MODE_CONNECTOR_DPI,
+   .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE,
+};
+
 static const struct display_timing multi_inno_mi0700s4t_6_timing = {
.pixelclock = { 2900, 3300, 3800 },
.hactive = { 800, 800, 800 },
@@ -4158,6 +4184,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "mitsubishi,aa070mc01-ca1",
.data = _aa070mc01,
+   }, {
+   .compatible = "mitsubishi,aa084xe01",
+   .data = _aa084xe01,
}, {
.compatible = "multi-inno,mi0700s4t-6",
.data = _inno_mi0700s4t_6,
-- 
2.34.1



Re: [PATCH v2 2/2] drm/panel: simple: Add support for Mitsubishi AA084XE01

2023-06-22 Thread Miquel Raynal
Hello,

miquel.ray...@bootlin.com wrote on Mon, 19 Jun 2023 09:43:48 +0200:

> From: Thomas Weber 
> 
> Add support for the Mitsubishi AA084XE01 panel which is an 8.4 inch XGA
> TFT-LCD module for industrial use.
> 
> Link: https://www.mouser.fr/datasheet/2/274/aa084xe01_e-364171.pdf
> Signed-off-by: Thomas Weber 
> Signed-off-by: Miquel Raynal 
> ---
> 
> Changes in v2:
> * Lowered the clock to match the typical 65MHz frequency.
> * Added the connector type and the missing bus flags.
> * Collected an A-by tag.
> 
>  drivers/gpu/drm/panel/panel-simple.c | 29 
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c 
> b/drivers/gpu/drm/panel/panel-simple.c
> index 8a3b685c2fcc..963f3223c985 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -2670,6 +2670,32 @@ static const struct panel_desc mitsubishi_aa070mc01 = {
>   .bus_flags = DRM_BUS_FLAG_DE_HIGH,
>  };
>  
> +static const struct drm_display_mode mitsubishi_aa084xe01_mode = {
> + .clock = 56234,
> + .hdisplay = 1024,
> + .hsync_start = 1024 + 24,
> + .hsync_end = 1024 + 24 + 63,
> + .htotal = 1024 + 24 + 63 + 1,
> + .vdisplay = 768,
> + .vsync_start = 768 + 3,
> + .vsync_end = 768 + 3 + 6,
> + .vtotal = 768 + 3 + 6 + 1,
> + .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
> +};
> +
> +static const struct panel_desc mitsubishi_aa084xe01 = {
> + .modes = _aa084xe01_mode,
> + .num_modes = 1,
> + .bpc = 8,
> + .size = {
> + .width = 1024,
> + .height = 768,
> + },
> + .bus_format = MEDIA_BUS_FMT_RGB565_1X16,
> + .connector_type = DRM_MODE_CONNECTOR_LVDS,

I've got confused, the connector is a DPI connector. On my board there
are two RGB-LVDS and LVDS-RGB connectors, but this is not relevant
here. I'll send an update using the right connector type.

> + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE,
> +};
> +
>  static const struct display_timing multi_inno_mi0700s4t_6_timing = {
>   .pixelclock = { 2900, 3300, 3800 },
>   .hactive = { 800, 800, 800 },
> @@ -4158,6 +4184,9 @@ static const struct of_device_id platform_of_match[] = {
>   }, {
>   .compatible = "mitsubishi,aa070mc01-ca1",
>   .data = _aa070mc01,
> + }, {
> + .compatible = "mitsubishi,aa084xe01",
> + .data = _aa084xe01,
>   }, {
>   .compatible = "multi-inno,mi0700s4t-6",
>   .data = _inno_mi0700s4t_6,


Thanks,
Miquèl


[PATCH v3 6/6] drm/panel: sitronix-st7789v: Check display ID

2023-06-19 Thread Miquel Raynal
A very basic debugging rule when a device is connected for the first
time is to access a read-only register which contains known data in
order to ensure the communication protocol is properly working. This
driver lacked any read helper which is often a critical piece for
speeding-up bring-ups.

Add a read helper and use it to verify the communication with the panel
is working as soon as possible in order to inform the user early if this
is not the case.

As this panel may work with no MISO line, the check is discarded in this
case. Upon error, we do not fail probing but just warn the user, in case
the DT description would be lacking the Rx bus width (which is likely on
old descriptions) in order to avoid breaking existing devices.

Signed-off-by: Miquel Raynal 
Acked-by: Sam Ravnborg 
Acked-by: Maxime Ripard 
---
 .../gpu/drm/panel/panel-sitronix-st7789v.c| 81 +++
 1 file changed, 81 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index 8649966ceae8..205de179f7f2 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -110,6 +110,9 @@
return val; \
} while (0)
 
+#define ST7789V_IDS { 0x85, 0x85, 0x52 }
+#define ST7789V_IDS_SIZE 3
+
 struct st7789_panel_info {
const struct drm_display_mode *mode;
u32 bus_format;
@@ -157,6 +160,76 @@ static int st7789v_write_data(struct st7789v *ctx, u8 cmd)
return st7789v_spi_write(ctx, ST7789V_DATA, cmd);
 }
 
+static int st7789v_read_data(struct st7789v *ctx, u8 cmd, u8 *buf,
+unsigned int len)
+{
+   struct spi_transfer xfer[2] = { };
+   struct spi_message msg;
+   u16 txbuf = ((ST7789V_COMMAND & 1) << 8) | cmd;
+   u16 rxbuf[4] = {};
+   u8 bit9 = 0;
+   int ret, i;
+
+   switch (len) {
+   case 1:
+   case 3:
+   case 4:
+   break;
+   default:
+   return -EOPNOTSUPP;
+   }
+
+   spi_message_init();
+
+   xfer[0].tx_buf = 
+   xfer[0].len = sizeof(txbuf);
+   spi_message_add_tail([0], );
+
+   xfer[1].rx_buf = rxbuf;
+   xfer[1].len = len * 2;
+   spi_message_add_tail([1], );
+
+   ret = spi_sync(ctx->spi, );
+   if (ret)
+   return ret;
+
+   for (i = 0; i < len; i++) {
+   buf[i] = rxbuf[i] >> i | (bit9 << (9 - i));
+   if (i)
+   bit9 = rxbuf[i] & GENMASK(i - 1, 0);
+   }
+
+   return 0;
+}
+
+static int st7789v_check_id(struct drm_panel *panel)
+{
+   const u8 st7789v_ids[ST7789V_IDS_SIZE] = ST7789V_IDS;
+   struct st7789v *ctx = panel_to_st7789v(panel);
+   bool invalid_ids = false;
+   int ret, i;
+   u8 ids[3];
+
+   if (ctx->spi->mode & SPI_NO_RX)
+   return 0;
+
+   ret = st7789v_read_data(ctx, MIPI_DCS_GET_DISPLAY_ID, ids, 
ST7789V_IDS_SIZE);
+   if (ret)
+   return ret;
+
+   for (i = 0; i < ST7789V_IDS_SIZE; i++) {
+   if (ids[i] != st7789v_ids[i]) {
+   invalid_ids = true;
+   break;
+   }
+   }
+
+   if (invalid_ids)
+   return -EIO;
+
+   return 0;
+}
+
 static const struct drm_display_mode default_mode = {
.clock = 7000,
.hdisplay = 240,
@@ -295,6 +368,14 @@ static int st7789v_prepare(struct drm_panel *panel)
gpiod_set_value(ctx->reset, 0);
msleep(120);
 
+   /*
+* Avoid failing if the IDs are invalid in case the Rx bus width
+* description is missing.
+*/
+   ret = st7789v_check_id(panel);
+   if (ret)
+   dev_warn(panel->dev, "Unrecognized panel IDs");
+
ST7789V_TEST(ret, st7789v_write_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE));
 
/* We need to wait 120ms after a sleep out command */
-- 
2.34.1



[PATCH v3 4/6] drm/panel: sitronix-st7789v: Clarify a definition

2023-06-19 Thread Miquel Raynal
The Sitronix datasheet explains BIT(1) of the RGBCTRL register as the
DOTCLK/PCLK edge used to sample the data lines:

“0” The data is input on the positive edge of DOTCLK
“1” The data is input on the negative edge of DOTCLK

IOW, this bit implies a falling edge and not a high state. Correct the
definition to ease the comparison with the datasheet.

Signed-off-by: Miquel Raynal 
Acked-by: Maxime Ripard 
---
 drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index 605b9f6d0f14..d7c5b3ad1baa 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -27,7 +27,7 @@
 #define ST7789V_RGBCTRL_RCM(n) (((n) & 3) << 5)
 #define ST7789V_RGBCTRL_VSYNC_HIGH BIT(3)
 #define ST7789V_RGBCTRL_HSYNC_HIGH BIT(2)
-#define ST7789V_RGBCTRL_PCLK_HIGH  BIT(1)
+#define ST7789V_RGBCTRL_PCLK_FALLING   BIT(1)
 #define ST7789V_RGBCTRL_DE_LOW BIT(0)
 #define ST7789V_RGBCTRL_VBP(n) ((n) & 0x7f)
 #define ST7789V_RGBCTRL_HBP(n) ((n) & 0x1f)
@@ -259,7 +259,7 @@ static int st7789v_prepare(struct drm_panel *panel)
if (ctx->info->mode->flags & DRM_MODE_FLAG_PHSYNC)
polarity |= ST7789V_RGBCTRL_HSYNC_HIGH;
if (ctx->info->bus_flags & DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE)
-   polarity |= ST7789V_RGBCTRL_PCLK_HIGH;
+   polarity |= ST7789V_RGBCTRL_PCLK_FALLING;
if (ctx->info->bus_flags & DRM_BUS_FLAG_DE_LOW)
polarity |= ST7789V_RGBCTRL_DE_LOW;
 
-- 
2.34.1



[PATCH v3 5/6] drm/panel: sitronix-st7789v: Add EDT ET028013DMA panel support

2023-06-19 Thread Miquel Raynal
This panel from Emerging Display Technologies Corporation features an
ST7789V2 LCD controller panel inside which is almost identical to what
the Sitronix panel driver supports.

In practice, the module physical size is specific, and experiments show
that the display will malfunction if any of the following situation
occurs:
* Pixel clock is above 3MHz
* Pixel clock is not inverted
I could not properly identify the reasons behind these failures, scope
captures show valid input signals.

Signed-off-by: Miquel Raynal 
Acked-by: Maxime Ripard 
---
 .../gpu/drm/panel/panel-sitronix-st7789v.c| 25 +++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index d7c5b3ad1baa..8649966ceae8 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -187,6 +187,21 @@ static const struct drm_display_mode t28cp45tn89_mode = {
.flags = DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC,
 };
 
+static const struct drm_display_mode et028013dma_mode = {
+   .clock = 3000,
+   .hdisplay = 240,
+   .hsync_start = 240 + 38,
+   .hsync_end = 240 + 38 + 10,
+   .htotal = 240 + 38 + 10 + 10,
+   .vdisplay = 320,
+   .vsync_start = 320 + 8,
+   .vsync_end = 320 + 8 + 4,
+   .vtotal = 320 + 8 + 4 + 4,
+   .width_mm = 43,
+   .height_mm = 58,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
+};
+
 struct st7789_panel_info default_panel = {
.mode = _mode,
.invert_mode = true,
@@ -203,6 +218,14 @@ struct st7789_panel_info t28cp45tn89_panel = {
 DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
 };
 
+struct st7789_panel_info et028013dma_panel = {
+   .mode = _mode,
+   .invert_mode = true,
+   .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
+   .bus_flags = DRM_BUS_FLAG_DE_HIGH |
+DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
+};
+
 static int st7789v_get_modes(struct drm_panel *panel,
 struct drm_connector *connector)
 {
@@ -474,6 +497,7 @@ static void st7789v_remove(struct spi_device *spi)
 static const struct spi_device_id st7789v_spi_id[] = {
{ "st7789v", (unsigned long) _panel },
{ "t28cp45tn89-v17", (unsigned long) _panel },
+   { "et028013dma", (unsigned long) _panel },
{ }
 };
 MODULE_DEVICE_TABLE(spi, st7789v_spi_id);
@@ -481,6 +505,7 @@ MODULE_DEVICE_TABLE(spi, st7789v_spi_id);
 static const struct of_device_id st7789v_of_match[] = {
{ .compatible = "sitronix,st7789v", .data = _panel },
{ .compatible = "inanbo,t28cp45tn89-v17", .data = _panel },
+   { .compatible = "edt,et028013dma", .data = _panel },
{ }
 };
 MODULE_DEVICE_TABLE(of, st7789v_of_match);
-- 
2.34.1



[PATCH v3 3/6] drm/panel: sitronix-st7789v: Use 9 bits per spi word by default

2023-06-19 Thread Miquel Raynal
The Sitronix controller expects 9-bit words, provide this as default at
probe time rather than specifying this in each and every access.

Signed-off-by: Miquel Raynal 
Reviewed-by: Sam Ravnborg 
Acked-by: Maxime Ripard 
---
 drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index 172c6c1fc090..605b9f6d0f14 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -142,7 +142,6 @@ static int st7789v_spi_write(struct st7789v *ctx, enum 
st7789v_prefix prefix,
u16 txbuf = ((prefix & 1) << 8) | data;
 
xfer.tx_buf = 
-   xfer.bits_per_word = 9;
xfer.len = sizeof(txbuf);
 
return spi_sync_transfer(ctx->spi, , 1);
@@ -436,6 +435,11 @@ static int st7789v_probe(struct spi_device *spi)
spi_set_drvdata(spi, ctx);
ctx->spi = spi;
 
+   spi->bits_per_word = 9;
+   ret = spi_setup(spi);
+   if (ret < 0)
+   return dev_err_probe(>dev, ret, "Failed to setup spi\n");
+
ctx->info = device_get_match_data(>dev);
 
drm_panel_init(>panel, dev, _drm_funcs,
-- 
2.34.1



[PATCH v3 2/6] dt-bindings: display: st7789v: bound the number of Rx data lines

2023-06-19 Thread Miquel Raynal
The ST7789V LCD controller supports regular SPI wiring, as well as no Rx
data line at all. The operating system needs to know whether it can read
registers from the device or not. Let's detail this specific design
possibility by bounding the spi-rx-bus-width property.

Signed-off-by: Miquel Raynal 
Acked-by: Krzysztof Kozlowski 
---
 .../devicetree/bindings/display/panel/sitronix,st7789v.yaml   | 4 
 1 file changed, 4 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml 
b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
index 0ccf0487fd8e..a25df7e1df88 100644
--- a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
+++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
@@ -29,6 +29,10 @@ properties:
   spi-cpha: true
   spi-cpol: true
 
+  spi-rx-bus-width:
+minimum: 0
+maximum: 1
+
 required:
   - compatible
   - reg
-- 
2.34.1



[PATCH v3 0/6] drm/panel: sitronix-st7789v: Support ET028013DMA panel

2023-06-19 Thread Miquel Raynal
Hello,

The aim of this series is to add support for the EDT ET028013DMA
panel. This panel features a Sitronix ST7789V2 LCD controller, which is
already supported mainline (or very close to the ST7789V for which
Maxime added support years ago).

The EDT panel is slightly different on the geometry and appears not to
support refresh rates higher than 30fps (above, glitches are visible,
despite the incoming signals being rather clean). While I was working on
this panel, I found quite inconvenient to not be able to read anything
back as it is a great tool for debugging purposes. So the last patch
actually adds a read helper and uses it to perform a sanity check at
probe time by verifying the Sitronix controller IDs.

This series applies on top of Sebastian's series which was also bringing
a number of good improvements to this driver. As Sebastian started and
contributed his work before me, I think his series is close to be merged
so I adapted my changes on top of it.

Link: https://lore.kernel.org/dri-devel/20230422205012.2464933-1-...@kernel.org/

Thanks,
Miquèl

Changes in v3:
* Following the exchanges with Maxime, existing devices will not stop
  probing if the IDs are wrong, just because old description might
  actually miss the Rx bus width DT parameter.
* Collected tags.

Changes in v2:
* Rebased on top of Sebastian's series and adapted all my changes to the
  existing infrastructure he has already added.
* Collected tags.
* Prevented the ID check to fail if there is no MISO line.
* Used dev_err_probe() when relevant.
* Sorted the IDs in the tables.
* Renamed the panel mode.
* Fixed typos.

Miquel Raynal (6):
  dt-bindings: display: st7789v: Add the edt,et028013dma panel
compatible
  dt-bindings: display: st7789v: bound the number of Rx data lines
  drm/panel: sitronix-st7789v: Use 9 bits per spi word by default
  drm/panel: sitronix-st7789v: Clarify a definition
  drm/panel: sitronix-st7789v: Add EDT ET028013DMA panel support
  drm/panel: sitronix-st7789v: Check display ID

 .../display/panel/sitronix,st7789v.yaml   |   5 +
 .../gpu/drm/panel/panel-sitronix-st7789v.c| 116 +-
 2 files changed, 118 insertions(+), 3 deletions(-)

-- 
2.34.1



[PATCH v3 1/6] dt-bindings: display: st7789v: Add the edt, et028013dma panel compatible

2023-06-19 Thread Miquel Raynal
The ST7789V LCD controller is also embedded in the ET028013DMA
panel. Add a compatible string to describe this other panel.

Signed-off-by: Miquel Raynal 
Acked-by: Krzysztof Kozlowski 
Acked-by: Maxime Ripard 
---
 .../devicetree/bindings/display/panel/sitronix,st7789v.yaml  | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml 
b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
index 7c5e4313db1d..0ccf0487fd8e 100644
--- a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
+++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
@@ -16,6 +16,7 @@ allOf:
 properties:
   compatible:
 enum:
+  - edt,et028013dma
   - inanbo,t28cp45tn89-v17
   - sitronix,st7789v
 
-- 
2.34.1



Re: [PATCH v2 2/6] dt-bindings: display: st7789v: bound the number of Rx data lines

2023-06-19 Thread Miquel Raynal
Hi Maxime,

max...@cerno.tech wrote on Mon, 19 Jun 2023 11:39:56 +0200:

> On Sun, Jun 18, 2023 at 07:37:32PM +0200, Miquel Raynal wrote:
> > Hello Maxime,
> > 
> > max...@cerno.tech wrote on Sun, 18 Jun 2023 16:37:58 +0200:
> >   
> > > Hi,
> > > 
> > > On Fri, Jun 16, 2023 at 06:32:51PM +0200, Miquel Raynal wrote:  
> > > > The ST7789V LCD controller supports regular SPI wiring, as well as no Rx
> > > > data line at all. The operating system needs to know whether it can read
> > > > registers from the device or not. Let's detail this specific design
> > > > possibility by bounding the spi-rx-bus-width property.
> > > > 
> > > > Signed-off-by: Miquel Raynal 
> > > > ---
> > > >  .../devicetree/bindings/display/panel/sitronix,st7789v.yaml   | 4 
> > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git 
> > > > a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml 
> > > > b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
> > > > index 0ccf0487fd8e..a25df7e1df88 100644
> > > > --- 
> > > > a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
> > > > +++ 
> > > > b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
> > > > @@ -29,6 +29,10 @@ properties:
> > > >spi-cpha: true
> > > >spi-cpol: true
> > > >  
> > > > +  spi-rx-bus-width:
> > > > +minimum: 0
> > > > +maximum: 1
> > > > +
> > > 
> > > It's not clear to me what the default would be?  
> > 
> > This binding references spi-peripheral-props.yaml which sets the
> > default to 1, I believe it is sane to keep it that way?  
> 
> I'm not sure.
> 
> The driver didn't need RX before, and we didn't have any property that
> was expressing whether we had MISO in the device tree.
> 
> That means we had both devices with and without MISO expressed in the
> same way, the driver handling both (by ignoring MISO entirely).
> 
> With this patch, you now introduce a property that specifies whether
> MISO is connected or not, and defaults to MISO being there. And a later
> patch will use MISO if it's available.
> 
> This means that, while it's working fine for devices that had MISO
> connected, devices that didn't are assumed to have it, and the driver
> makes use of it.

Ah yes, I get your concern. I thought you wanted to talk about the fact
that it was not constrained in the yaml description. Your concern is
about breaking existing devices.

Your concern is real, designs not wiring the MISO line which do not
describe this in the device tree will no longer succeed to probe with
the current implementation. Technically speaking, they're broken since
2021:

c476d430bfc0 ("dt-bindings: display: Add SPI peripheral schema to SPI based 
displays")

We actually discussed this with Sebastian, right there:
https://lore.kernel.org/all/20230609145951.853533-6-miquel.ray...@bootlin.com/T/#m9286cdb4d617c5efc29052b552e981ecfa2628e4

And the conclusion was that we decided not to care about the broken
descriptions (because, let's agree on this, not wiring the MISO line is
not a standard spi design). But I don't have a strong opinion TBH, so
if you think it's best to prevent these probes from failing (note that
I already added a debug line explicitly saying why the probe would
fail, "easy" to identify) I'm fine turning the check as a warning and
ignoring the error to avoid failing.

Thanks,
Miquèl


[PATCH v2 1/2] dt-bindings: display: simple: Add Mitsubishi AA084XE01 panel

2023-06-19 Thread Miquel Raynal
Add Mitsubishi AA084XE01 8.4" XGA TFT LCD panel compatible string.

Signed-off-by: Miquel Raynal 
Acked-by: Krzysztof Kozlowski 
---
 .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml 
b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 18241f4051d2..cc841cf96fae 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -232,6 +232,8 @@ properties:
   - logictechno,lttd800480070-l6wh-rt
 # Mitsubishi "AA070MC01 7.0" WVGA TFT LCD panel
   - mitsubishi,aa070mc01-ca1
+# Mitsubishi AA084XE01 8.4" XGA TFT LCD panel
+  - mitsubishi,aa084xe01
 # Multi-Inno Technology Co.,Ltd MI0700S4T-6 7" 800x480 TFT Resistive 
Touch Module
   - multi-inno,mi0700s4t-6
 # Multi-Inno Technology Co.,Ltd MI0800FT-9 8" 800x600 TFT Resistive 
Touch Module
-- 
2.34.1



[PATCH v2 2/2] drm/panel: simple: Add support for Mitsubishi AA084XE01

2023-06-19 Thread Miquel Raynal
From: Thomas Weber 

Add support for the Mitsubishi AA084XE01 panel which is an 8.4 inch XGA
TFT-LCD module for industrial use.

Link: https://www.mouser.fr/datasheet/2/274/aa084xe01_e-364171.pdf
Signed-off-by: Thomas Weber 
Signed-off-by: Miquel Raynal 
---

Changes in v2:
* Lowered the clock to match the typical 65MHz frequency.
* Added the connector type and the missing bus flags.
* Collected an A-by tag.

 drivers/gpu/drm/panel/panel-simple.c | 29 
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 8a3b685c2fcc..963f3223c985 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2670,6 +2670,32 @@ static const struct panel_desc mitsubishi_aa070mc01 = {
.bus_flags = DRM_BUS_FLAG_DE_HIGH,
 };
 
+static const struct drm_display_mode mitsubishi_aa084xe01_mode = {
+   .clock = 56234,
+   .hdisplay = 1024,
+   .hsync_start = 1024 + 24,
+   .hsync_end = 1024 + 24 + 63,
+   .htotal = 1024 + 24 + 63 + 1,
+   .vdisplay = 768,
+   .vsync_start = 768 + 3,
+   .vsync_end = 768 + 3 + 6,
+   .vtotal = 768 + 3 + 6 + 1,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
+};
+
+static const struct panel_desc mitsubishi_aa084xe01 = {
+   .modes = _aa084xe01_mode,
+   .num_modes = 1,
+   .bpc = 8,
+   .size = {
+   .width = 1024,
+   .height = 768,
+   },
+   .bus_format = MEDIA_BUS_FMT_RGB565_1X16,
+   .connector_type = DRM_MODE_CONNECTOR_LVDS,
+   .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE,
+};
+
 static const struct display_timing multi_inno_mi0700s4t_6_timing = {
.pixelclock = { 2900, 3300, 3800 },
.hactive = { 800, 800, 800 },
@@ -4158,6 +4184,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "mitsubishi,aa070mc01-ca1",
.data = _aa070mc01,
+   }, {
+   .compatible = "mitsubishi,aa084xe01",
+   .data = _aa084xe01,
}, {
.compatible = "multi-inno,mi0700s4t-6",
.data = _inno_mi0700s4t_6,
-- 
2.34.1



Re: [PATCH v2 2/6] dt-bindings: display: st7789v: bound the number of Rx data lines

2023-06-18 Thread Miquel Raynal
Hello Maxime,

max...@cerno.tech wrote on Sun, 18 Jun 2023 16:37:58 +0200:

> Hi,
> 
> On Fri, Jun 16, 2023 at 06:32:51PM +0200, Miquel Raynal wrote:
> > The ST7789V LCD controller supports regular SPI wiring, as well as no Rx
> > data line at all. The operating system needs to know whether it can read
> > registers from the device or not. Let's detail this specific design
> > possibility by bounding the spi-rx-bus-width property.
> > 
> > Signed-off-by: Miquel Raynal 
> > ---
> >  .../devicetree/bindings/display/panel/sitronix,st7789v.yaml   | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml 
> > b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
> > index 0ccf0487fd8e..a25df7e1df88 100644
> > --- a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
> > @@ -29,6 +29,10 @@ properties:
> >spi-cpha: true
> >spi-cpol: true
> >  
> > +  spi-rx-bus-width:
> > +minimum: 0
> > +maximum: 1
> > +  
> 
> It's not clear to me what the default would be?

This binding references spi-peripheral-props.yaml which sets the
default to 1, I believe it is sane to keep it that way?

Thanks,
Miquèl


[PATCH 2/2] drm/panel: simple: Add support for Mitsubishi AA084XE01

2023-06-16 Thread Miquel Raynal
From: Thomas Weber 

Add support for the Mitsubishi AA084XE01 panel which is an 8.4 inch XGA
TFT-LCD module for industrial use.

Link: https://www.mouser.fr/datasheet/2/274/aa084xe01_e-364171.pdf
Signed-off-by: Thomas Weber 
Signed-off-by: Miquel Raynal 
---
 drivers/gpu/drm/panel/panel-simple.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 8a3b685c2fcc..f79c9f9124a0 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2670,6 +2670,30 @@ static const struct panel_desc mitsubishi_aa070mc01 = {
.bus_flags = DRM_BUS_FLAG_DE_HIGH,
 };
 
+static const struct drm_display_mode mitsubishi_aa084xe01_mode = {
+   .clock = 8,
+   .hdisplay = 1024,
+   .hsync_start = 1024 + 24,
+   .hsync_end = 1024 + 24 + 63,
+   .htotal = 1024 + 24 + 63 + 1,
+   .vdisplay = 768,
+   .vsync_start = 768 + 3,
+   .vsync_end = 768 + 3 + 6,
+   .vtotal = 768 + 3 + 6 + 1,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
+};
+
+static const struct panel_desc mitsubishi_aa084xe01 = {
+   .modes = _aa084xe01_mode,
+   .num_modes = 1,
+   .bpc = 8,
+   .size = {
+   .width = 1024,
+   .height = 768,
+   },
+   .bus_format = MEDIA_BUS_FMT_RGB565_1X16,
+};
+
 static const struct display_timing multi_inno_mi0700s4t_6_timing = {
.pixelclock = { 2900, 3300, 3800 },
.hactive = { 800, 800, 800 },
@@ -4158,6 +4182,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "mitsubishi,aa070mc01-ca1",
.data = _aa070mc01,
+   }, {
+   .compatible = "mitsubishi,aa084xe01",
+   .data = _aa084xe01,
}, {
.compatible = "multi-inno,mi0700s4t-6",
.data = _inno_mi0700s4t_6,
-- 
2.34.1



[PATCH 1/2] dt-bindings: display: simple: Add Mitsubishi AA084XE01 panel

2023-06-16 Thread Miquel Raynal
Add Mitsubishi AA084XE01 8.4" XGA TFT LCD panel compatible string.

Signed-off-by: Miquel Raynal 
---
 .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml 
b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 18241f4051d2..cc841cf96fae 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -232,6 +232,8 @@ properties:
   - logictechno,lttd800480070-l6wh-rt
 # Mitsubishi "AA070MC01 7.0" WVGA TFT LCD panel
   - mitsubishi,aa070mc01-ca1
+# Mitsubishi AA084XE01 8.4" XGA TFT LCD panel
+  - mitsubishi,aa084xe01
 # Multi-Inno Technology Co.,Ltd MI0700S4T-6 7" 800x480 TFT Resistive 
Touch Module
   - multi-inno,mi0700s4t-6
 # Multi-Inno Technology Co.,Ltd MI0800FT-9 8" 800x600 TFT Resistive 
Touch Module
-- 
2.34.1



[PATCH v2 5/6] drm/panel: sitronix-st7789v: Add EDT ET028013DMA panel support

2023-06-16 Thread Miquel Raynal
This panel from Emerging Display Technologies Corporation features an
ST7789V2 LCD controller panel inside which is almost identical to what
the Sitronix panel driver supports.

In practice, the module physical size is specific, and experiments show
that the display will malfunction if any of the following situation
occurs:
* Pixel clock is above 3MHz
* Pixel clock is not inverted
I could not properly identify the reasons behind these failures, scope
captures show valid input signals.

Signed-off-by: Miquel Raynal 
---
 .../gpu/drm/panel/panel-sitronix-st7789v.c| 25 +++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index d7c5b3ad1baa..8649966ceae8 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -187,6 +187,21 @@ static const struct drm_display_mode t28cp45tn89_mode = {
.flags = DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC,
 };
 
+static const struct drm_display_mode et028013dma_mode = {
+   .clock = 3000,
+   .hdisplay = 240,
+   .hsync_start = 240 + 38,
+   .hsync_end = 240 + 38 + 10,
+   .htotal = 240 + 38 + 10 + 10,
+   .vdisplay = 320,
+   .vsync_start = 320 + 8,
+   .vsync_end = 320 + 8 + 4,
+   .vtotal = 320 + 8 + 4 + 4,
+   .width_mm = 43,
+   .height_mm = 58,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
+};
+
 struct st7789_panel_info default_panel = {
.mode = _mode,
.invert_mode = true,
@@ -203,6 +218,14 @@ struct st7789_panel_info t28cp45tn89_panel = {
 DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
 };
 
+struct st7789_panel_info et028013dma_panel = {
+   .mode = _mode,
+   .invert_mode = true,
+   .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
+   .bus_flags = DRM_BUS_FLAG_DE_HIGH |
+DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
+};
+
 static int st7789v_get_modes(struct drm_panel *panel,
 struct drm_connector *connector)
 {
@@ -474,6 +497,7 @@ static void st7789v_remove(struct spi_device *spi)
 static const struct spi_device_id st7789v_spi_id[] = {
{ "st7789v", (unsigned long) _panel },
{ "t28cp45tn89-v17", (unsigned long) _panel },
+   { "et028013dma", (unsigned long) _panel },
{ }
 };
 MODULE_DEVICE_TABLE(spi, st7789v_spi_id);
@@ -481,6 +505,7 @@ MODULE_DEVICE_TABLE(spi, st7789v_spi_id);
 static const struct of_device_id st7789v_of_match[] = {
{ .compatible = "sitronix,st7789v", .data = _panel },
{ .compatible = "inanbo,t28cp45tn89-v17", .data = _panel },
+   { .compatible = "edt,et028013dma", .data = _panel },
{ }
 };
 MODULE_DEVICE_TABLE(of, st7789v_of_match);
-- 
2.34.1



[PATCH v2 3/6] drm/panel: sitronix-st7789v: Use 9 bits per spi word by default

2023-06-16 Thread Miquel Raynal
The Sitronix controller expects 9-bit words, provide this as default at
probe time rather than specifying this in each and every access.

Signed-off-by: Miquel Raynal 
Reviewed-by: Sam Ravnborg 
---
 drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index 172c6c1fc090..605b9f6d0f14 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -142,7 +142,6 @@ static int st7789v_spi_write(struct st7789v *ctx, enum 
st7789v_prefix prefix,
u16 txbuf = ((prefix & 1) << 8) | data;
 
xfer.tx_buf = 
-   xfer.bits_per_word = 9;
xfer.len = sizeof(txbuf);
 
return spi_sync_transfer(ctx->spi, , 1);
@@ -436,6 +435,11 @@ static int st7789v_probe(struct spi_device *spi)
spi_set_drvdata(spi, ctx);
ctx->spi = spi;
 
+   spi->bits_per_word = 9;
+   ret = spi_setup(spi);
+   if (ret < 0)
+   return dev_err_probe(>dev, ret, "Failed to setup spi\n");
+
ctx->info = device_get_match_data(>dev);
 
drm_panel_init(>panel, dev, _drm_funcs,
-- 
2.34.1



[PATCH v2 4/6] drm/panel: sitronix-st7789v: Clarify a definition

2023-06-16 Thread Miquel Raynal
The Sitronix datasheet explains BIT(1) of the RGBCTRL register as the
DOTCLK/PCLK edge used to sample the data lines:

“0” The data is input on the positive edge of DOTCLK
“1” The data is input on the negative edge of DOTCLK

IOW, this bit implies a falling edge and not a high state. Correct the
definition to ease the comparison with the datasheet.

Signed-off-by: Miquel Raynal 
---
 drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index 605b9f6d0f14..d7c5b3ad1baa 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -27,7 +27,7 @@
 #define ST7789V_RGBCTRL_RCM(n) (((n) & 3) << 5)
 #define ST7789V_RGBCTRL_VSYNC_HIGH BIT(3)
 #define ST7789V_RGBCTRL_HSYNC_HIGH BIT(2)
-#define ST7789V_RGBCTRL_PCLK_HIGH  BIT(1)
+#define ST7789V_RGBCTRL_PCLK_FALLING   BIT(1)
 #define ST7789V_RGBCTRL_DE_LOW BIT(0)
 #define ST7789V_RGBCTRL_VBP(n) ((n) & 0x7f)
 #define ST7789V_RGBCTRL_HBP(n) ((n) & 0x1f)
@@ -259,7 +259,7 @@ static int st7789v_prepare(struct drm_panel *panel)
if (ctx->info->mode->flags & DRM_MODE_FLAG_PHSYNC)
polarity |= ST7789V_RGBCTRL_HSYNC_HIGH;
if (ctx->info->bus_flags & DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE)
-   polarity |= ST7789V_RGBCTRL_PCLK_HIGH;
+   polarity |= ST7789V_RGBCTRL_PCLK_FALLING;
if (ctx->info->bus_flags & DRM_BUS_FLAG_DE_LOW)
polarity |= ST7789V_RGBCTRL_DE_LOW;
 
-- 
2.34.1



[PATCH v2 2/6] dt-bindings: display: st7789v: bound the number of Rx data lines

2023-06-16 Thread Miquel Raynal
The ST7789V LCD controller supports regular SPI wiring, as well as no Rx
data line at all. The operating system needs to know whether it can read
registers from the device or not. Let's detail this specific design
possibility by bounding the spi-rx-bus-width property.

Signed-off-by: Miquel Raynal 
---
 .../devicetree/bindings/display/panel/sitronix,st7789v.yaml   | 4 
 1 file changed, 4 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml 
b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
index 0ccf0487fd8e..a25df7e1df88 100644
--- a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
+++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
@@ -29,6 +29,10 @@ properties:
   spi-cpha: true
   spi-cpol: true
 
+  spi-rx-bus-width:
+minimum: 0
+maximum: 1
+
 required:
   - compatible
   - reg
-- 
2.34.1



[PATCH v2 6/6] drm/panel: sitronix-st7789v: Check display ID

2023-06-16 Thread Miquel Raynal
A very basic debugging rule when a device is connected for the first
time is to access a read-only register which contains known data in
order to ensure the communication protocol is properly working. This
driver lacked any read helper which is often a critical piece for
speed-up bring-ups.

Add a read helper and use it to verify the communication with the panel
is working as soon as possible in order to fail early if this is not the
case.

As this panel may work with no MISO line, the check is discarded in this
case.

Signed-off-by: Miquel Raynal 
Acked-by: Sam Ravnborg 
---
 .../gpu/drm/panel/panel-sitronix-st7789v.c| 81 +++
 1 file changed, 81 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index 8649966ceae8..36fb3f2453f1 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -110,6 +110,9 @@
return val; \
} while (0)
 
+#define ST7789V_IDS { 0x85, 0x85, 0x52 }
+#define ST7789V_IDS_SIZE 3
+
 struct st7789_panel_info {
const struct drm_display_mode *mode;
u32 bus_format;
@@ -157,6 +160,80 @@ static int st7789v_write_data(struct st7789v *ctx, u8 cmd)
return st7789v_spi_write(ctx, ST7789V_DATA, cmd);
 }
 
+static int st7789v_read_data(struct st7789v *ctx, u8 cmd, u8 *buf,
+unsigned int len)
+{
+   struct spi_transfer xfer[2] = { };
+   struct spi_message msg;
+   u16 txbuf = ((ST7789V_COMMAND & 1) << 8) | cmd;
+   u16 rxbuf[4] = {};
+   u8 bit9 = 0;
+   int ret, i;
+
+   switch (len) {
+   case 1:
+   case 3:
+   case 4:
+   break;
+   default:
+   return -EOPNOTSUPP;
+   }
+
+   spi_message_init();
+
+   xfer[0].tx_buf = 
+   xfer[0].len = sizeof(txbuf);
+   spi_message_add_tail([0], );
+
+   xfer[1].rx_buf = rxbuf;
+   xfer[1].len = len * 2;
+   spi_message_add_tail([1], );
+
+   ret = spi_sync(ctx->spi, );
+   if (ret)
+   return ret;
+
+   for (i = 0; i < len; i++) {
+   buf[i] = rxbuf[i] >> i | (bit9 << (9 - i));
+   if (i)
+   bit9 = rxbuf[i] & GENMASK(i - 1, 0);
+   }
+
+   return 0;
+}
+
+static int st7789v_check_id(struct drm_panel *panel)
+{
+   const u8 st7789v_ids[ST7789V_IDS_SIZE] = ST7789V_IDS;
+   struct st7789v *ctx = panel_to_st7789v(panel);
+   bool invalid_ids = false;
+   int ret, i;
+   u8 ids[3];
+
+   if (ctx->spi->mode & SPI_NO_RX)
+   return 0;
+
+   ret = st7789v_read_data(ctx, MIPI_DCS_GET_DISPLAY_ID, ids, 
ST7789V_IDS_SIZE);
+   if (ret) {
+   dev_err(panel->dev, "Failed to read IDs\n");
+   return ret;
+   }
+
+   for (i = 0; i < ST7789V_IDS_SIZE; i++) {
+   if (ids[i] != st7789v_ids[i]) {
+   invalid_ids = true;
+   break;
+   }
+   }
+
+   if (invalid_ids) {
+   dev_err(panel->dev, "Unrecognized panel IDs");
+   return -EIO;
+   }
+
+   return 0;
+}
+
 static const struct drm_display_mode default_mode = {
.clock = 7000,
.hdisplay = 240,
@@ -295,6 +372,10 @@ static int st7789v_prepare(struct drm_panel *panel)
gpiod_set_value(ctx->reset, 0);
msleep(120);
 
+   ret = st7789v_check_id(panel);
+   if (ret)
+   return ret;
+
ST7789V_TEST(ret, st7789v_write_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE));
 
/* We need to wait 120ms after a sleep out command */
-- 
2.34.1



[PATCH v2 0/6] drm/panel: sitronix-st7789v: Support ET028013DMA panel

2023-06-16 Thread Miquel Raynal
Hello,

The aim of this series is to add support for the EDT ET028013DMA
panel. This panel features a Sitronix ST7789V2 LCD controller, which is
already supported mainline (or very close to the ST7789V for which
Maxime added support years ago).

The EDT panel is slightly different on the geometry and appears not to
support refresh rates higher than 30fps (above, glitches are visible,
despite the incoming signals being rather clean). While I was working on
this panel, I found quite inconvenient to not be able to read anything
back as it is a great tool for debugging purposes. So the last patch
actually adds a read helper and uses it to perform a sanity check at
probe time by verifying the Sitronix controller IDs.

This series applies on top of Sebastian's series which was also bringing
a number of good improvements to this driver. As Sebastian started and
contributed his work before me, I think his series is close to be merged
so I adapted my changes on top of it.

Link: https://lore.kernel.org/dri-devel/20230422205012.2464933-1-...@kernel.org/

Thanks,
Miquèl

Changes in v2:
* Rebased on top of Sebastian's series and adapted all my changes to the
  existing infrastructure he has already added.
* Collected tags.
* Prevented the ID check to fail if there is no MISO line.
* Used dev_err_probe() when relevant.
* Sorted the IDs in the tables.
* Renamed the panel mode.
* Fixed typos.

Miquel Raynal (6):
  dt-bindings: display: st7789v: Add the edt,et028013dma panel
compatible
  dt-bindings: display: st7789v: bound the number of Rx data lines
  drm/panel: sitronix-st7789v: Use 9 bits per spi word by default
  drm/panel: sitronix-st7789v: Clarify a definition
  drm/panel: sitronix-st7789v: Add EDT ET028013DMA panel support
  drm/panel: sitronix-st7789v: Check display ID

 .../display/panel/sitronix,st7789v.yaml   |   5 +
 .../gpu/drm/panel/panel-sitronix-st7789v.c| 116 +-
 2 files changed, 118 insertions(+), 3 deletions(-)

-- 
2.34.1



[PATCH v2 1/6] dt-bindings: display: st7789v: Add the edt, et028013dma panel compatible

2023-06-16 Thread Miquel Raynal
The ST7789V LCD controller is also embedded in the ET028013DMA
panel. Add a compatible string to describe this other panel.

Signed-off-by: Miquel Raynal 
---
 .../devicetree/bindings/display/panel/sitronix,st7789v.yaml  | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml 
b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
index 7c5e4313db1d..0ccf0487fd8e 100644
--- a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
+++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
@@ -16,6 +16,7 @@ allOf:
 properties:
   compatible:
 enum:
+  - edt,et028013dma
   - inanbo,t28cp45tn89-v17
   - sitronix,st7789v
 
-- 
2.34.1



Re: [PATCH 7/7] drm/panel: sitronix-st7789v: Check display ID

2023-06-16 Thread Miquel Raynal
Hi Sebastian,

s...@kernel.org wrote on Thu, 15 Jun 2023 01:27:24 +0200:

> Hi,
> 
> On Sat, Jun 10, 2023 at 10:45:25PM +0200, Sam Ravnborg wrote:
> > Hi Miquel,
> > 
> > On Fri, Jun 09, 2023 at 04:59:51PM +0200, Miquel Raynal wrote:  
> > > A very basic debugging rule when a device is connected for the first
> > > time is to access a read-only register which contains known data in
> > > order to ensure the communication protocol is properly working. This
> > > driver lacked any read helper which is often a critical peace for
> > > fastening bring-ups.
> > > 
> > > Add a read helper and use it to verify the communication with the panel
> > > is working as soon as possible in order to fail early if this is not the
> > > case.  
> > 
> > The read helper seems like a log of general boiler plate code.
> > I briefly looked into the use of regmap for the spi communication,
> > but it did not look like it supported the bit9 stuff.
> > 
> > I did not stare enough to add a reviewd by, but the approach is fine
> > and it is good to detech issues early.  
> 
> The st7789v datasheet describes a setup where SPI is connected
> unidirectional (i.e. without MISO). In that case the ID check
> will fail.

Right. I'll add a (spi->mode & SPI_NO_RX) check, as the default is to
have both lines, if there is no MISO line, I'd expect it to be
described in the DT, otherwise the description is broken.

Thanks,
Miquèl


Re: [PATCH 6/7] drm/panel: sitronix-st7789v: Add EDT ET028013DMA panel support

2023-06-16 Thread Miquel Raynal
Hi Maxime,

mrip...@kernel.org wrote on Mon, 12 Jun 2023 10:51:09 +0200:

> On Fri, Jun 09, 2023 at 04:59:50PM +0200, Miquel Raynal wrote:
> > This panel from Emerging Display Technologies Corporation features an
> > ST7789V2 panel inside which is almost identical to what the Sitronix
> > panel driver supports.
> > 
> > In practice, the module physical size is specific, and experiments show
> > that the display will malfunction if any of the following situation
> > occurs:
> > * Pixel clock is above 3MHz
> > * Pixel clock is not inverted
> > I could not properly identify the reasons behind these failures, scope
> > captures show valid input signals.
> > 
> > Signed-off-by: Miquel Raynal 
> > ---
> >  .../gpu/drm/panel/panel-sitronix-st7789v.c| 34 +--
> >  1 file changed, 32 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
> > b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > index 212bccc31804..7de192a3a8aa 100644
> > --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > @@ -30,7 +30,8 @@
> >  #define ST7789V_RGBCTRL_RCM(n) (((n) & 3) << 5)
> >  #define ST7789V_RGBCTRL_VSYNC_HIGH BIT(3)
> >  #define ST7789V_RGBCTRL_HSYNC_HIGH BIT(2)
> > -#define ST7789V_RGBCTRL_PCLK_HIGH  BIT(1)
> > +#define ST7789V_RGBCTRL_PCLK_FALLING   BIT(1)
> > +#define ST7789V_RGBCTRL_PCLK_RISING0
> >  #define ST7789V_RGBCTRL_VBP(n) ((n) & 0x7f)
> >  #define ST7789V_RGBCTRL_HBP(n) ((n) & 0x1f)
> >  
> > @@ -117,6 +118,7 @@ struct st7789v_panel_info {
> > u16 width_mm;
> > u16 height_mm;
> > u32 bus_format;
> > +   u32 bus_flags;
> >  };
> >  
> >  struct st7789v {
> > @@ -175,6 +177,18 @@ static const struct drm_display_mode default_mode = {
> > .vtotal = 320 + 8 + 4 + 4,
> >  };
> >  
> > +static const struct drm_display_mode slow_mode = {
> > +   .clock = 3000,
> > +   .hdisplay = 240,
> > +   .hsync_start = 240 + 38,
> > +   .hsync_end = 240 + 38 + 10,
> > +   .htotal = 240 + 38 + 10 + 10,
> > +   .vdisplay = 320,
> > +   .vsync_start = 320 + 8,
> > +   .vsync_end = 320 + 8 + 4,
> > +   .vtotal = 320 + 8 + 4 + 4,
> > +};  
> 
> Why is it supposed to be slow (and compared to what)? It looks like a
> fairly normal mode to me? Or is it because it's refreshed at 30Hz?

The initial support was using 60Hz and for a reason that I do not
understand (scope capture look right), the panel I use is highly
unstable at whatever frequency above 30Hz, so for me it was "slow" :-)
But of course I'll use the panel name to qualify the mode.

> Either way, the ST7789V is a panel controller and can be connected to a
> wide range of panels depending on the model, so it would be better to
> just use the model name there.

Sure.

> >  static int st7789v_get_modes(struct drm_panel *panel,
> >  struct drm_connector *connector)
> >  {
> > @@ -197,6 +211,7 @@ static int st7789v_get_modes(struct drm_panel *panel,
> >  
> > connector->display_info.width_mm = panel_info->width_mm;
> > connector->display_info.height_mm = panel_info->height_mm;
> > +   connector->display_info.bus_flags = panel_info->bus_flags;
> > drm_display_info_set_bus_formats(>display_info,
> >  _info->bus_format, 1);
> >  
> > @@ -206,8 +221,13 @@ static int st7789v_get_modes(struct drm_panel *panel,
> >  static int st7789v_prepare(struct drm_panel *panel)
> >  {
> > struct st7789v *ctx = panel_to_st7789v(panel);
> > +   const struct st7789v_panel_info *panel_info = ctx->panel_info;
> > +   u8 pck = ST7789V_RGBCTRL_PCLK_FALLING;
> > int ret;
> >  
> > +   if (panel_info->bus_flags & DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE)
> > +   pck = ST7789V_RGBCTRL_PCLK_RISING;
> > +  
> 
> I guess it could be in a separate commit

I will rebase on top of Sebastian's series who already addressed that
point (as well as many others).

[...]

> >  
> > +static const struct st7789v_panel_info et028013dma_info = {
> > +   .display_mode = _mode,
> > +   .width_mm = 43,
> > +   .height_mm = 58,
> > +   .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> > +   .bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
> > +};
> > +
> >  static const struct of_device_id st7789v_of_match[] = {
> > { .compatible = "sitronix,st7789v", .data = _info },
> > +   { .compatible = "edt,et028013dma", .data = _info },  
> 
> We should sort them by alphabetical order
> 
> Maxime

Ok!

Thanks,
Miquèl


Re: [PATCH 3/7] drm/panel: sitronix-st7789v: Specify the expected bus format

2023-06-16 Thread Miquel Raynal
Hi Sam,

s...@ravnborg.org wrote on Sat, 10 Jun 2023 22:12:46 +0200:

> On Fri, Jun 09, 2023 at 04:59:47PM +0200, Miquel Raynal wrote:
> > The LCD controller supports RGB444, RGB565 and RGB888. The value that is
> > written in the COLMOD register indicates using RGB888, so let's clearly
> > specify the in-use bus format.  
> 
> Confused.
> MEDIA_BUS_FMT_RGB666_1X18 assumes 6 bits per color.
> But RGB888 is 8 bits per color.
> 
> Something that I have forgotten, or is this inconsistent?

That is a typo indeed, I meant RBG666.

Thanks, Miquèl


Re: [PATCH v4] mtd: rawnand: macronix: OTP access for MX30LFxG18AC

2023-06-14 Thread Miquel Raynal
Hi liao,

jaimeliao...@gmail.com wrote on Wed, 14 Jun 2023 17:06:16 +0800:

> Hi Miquel
> 
> 
> >
> > Hello,
> >
> > avkras...@sberdevices.ru wrote on Tue, 23 May 2023 13:16:34 +0300:
> >  
> > > This adds support for OTP area access on MX30LFxG18AC chip series.  
> >
> > Jaime, any feedback on this? Will you test it?
> >
> > How are we supposed to test the OTP is locked? I see this is still an
> > open point.  
> After checking with internal, sub feature parameter are volatile register.
> 
> It could be change after enter/exit OTP region or power cycle even OTP
> 
> region have been locked.
> 
> OTP operation mode still could be enter/exit and region is read only
> after OTP in protect mode.
> 
> #program command could execute but no use after setting OTP region in
> protect mode.
> 
> So that we can't check whether OTP region is locked via get feature.
> 
> And we don't have region for checking status of OTP locked.

Ah, too bad. But thanks a lot for the explanation. Arseniy, can you
please change your comment to explain that the bit is volatile and thus
there is no way to check if an otp region is locked? I would return
EOPNOTSUPP in this case and verify that the core cleanly handles the
situation.

Thanks,
Miquèl

> 
> >  
> > >
> > > Signed-off-by: Arseniy Krasnov 
> > > ---
> > >   v1 -> v2:
> > >   * Add slab.h include due to kernel test robot error.
> > >   v2 -> v3:
> > >   * Use 'uint64_t' as input argument for 'do_div()' instead
> > > of 'unsigned long' due to kernel test robot error.
> > >   v3 -> v4:
> > >   * Use 'dev_err()' instead of 'WARN()'.
> > >   * Call 'match_string()' before checking 'supports_set_get_features'
> > > in 'macronix_nand_setup_otp().
> > >   * Use 'u8' instead of 'uint8_t' as ./checkpatch.pl wants.
> > >
> > >  drivers/mtd/nand/raw/nand_macronix.c | 216 +++
> > >  1 file changed, 216 insertions(+)
> > >
> > > diff --git a/drivers/mtd/nand/raw/nand_macronix.c 
> > > b/drivers/mtd/nand/raw/nand_macronix.c
> > > index 1472f925f386..be1ffa93bebb 100644
> > > --- a/drivers/mtd/nand/raw/nand_macronix.c
> > > +++ b/drivers/mtd/nand/raw/nand_macronix.c
> > > @@ -6,6 +6,7 @@
> > >   * Author: Boris Brezillon 
> > >   */
> > >
> > > +#include 
> > >  #include "linux/delay.h"
> > >  #include "internals.h"
> > >
> > > @@ -31,6 +32,20 @@
> > >
> > >  #define MXIC_CMD_POWER_DOWN 0xB9
> > >
> > > +#define ONFI_FEATURE_ADDR_30LFXG18AC_OTP 0x90
> > > +#define MACRONIX_30LFXG18AC_OTP_START_PAGE   0
> > > +#define MACRONIX_30LFXG18AC_OTP_PAGES30
> > > +#define MACRONIX_30LFXG18AC_OTP_PAGE_SIZE2112
> > > +#define MACRONIX_30LFXG18AC_OTP_START_BYTE   \
> > > + (MACRONIX_30LFXG18AC_OTP_START_PAGE *   \
> > > +  MACRONIX_30LFXG18AC_OTP_PAGE_SIZE)
> > > +#define MACRONIX_30LFXG18AC_OTP_SIZE_BYTES   \
> > > + (MACRONIX_30LFXG18AC_OTP_PAGES *\
> > > +  MACRONIX_30LFXG18AC_OTP_PAGE_SIZE)
> > > +
> > > +#define MACRONIX_30LFXG18AC_OTP_EN   BIT(0)
> > > +#define MACRONIX_30LFXG18AC_OTP_LOCKED   BIT(1)
> > > +
> > >  struct nand_onfi_vendor_macronix {
> > >   u8 reserved;
> > >   u8 reliability_func;
> > > @@ -316,6 +331,206 @@ static void 
> > > macronix_nand_deep_power_down_support(struct nand_chip *chip)
> > >   chip->ops.resume = mxic_nand_resume;
> > >  }
> > >
> > > +static int macronix_30lfxg18ac_get_otp_info(struct mtd_info *mtd, size_t 
> > > len,
> > > + size_t *retlen,
> > > + struct otp_info *buf)
> > > +{
> > > + if (len < sizeof(*buf))
> > > + return -EINVAL;
> > > +
> > > + /* Don't know how to check that OTP is locked. */
> > > + buf->locked = 0;
> > > + buf->start = MACRONIX_30LFXG18AC_OTP_START_BYTE;
> > > + buf->length = MACRONIX_30LFXG18AC_OTP_SIZE_BYTES;
> > > +
> > > + *retlen = sizeof(*buf);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int macronix_30lfxg18ac_otp_enable(struct nand_chip *nand)
> > > +{
> > > + u8 feature_buf[ONFI_SUBFEATURE_PARAM_LEN] = { 0 };
> > > +
> > > + feature_buf[0] = MACRONIX_30LFXG18AC_OTP_EN;
> > > + return nand_set_features(nand, ONFI_FEATURE_ADDR_30LFXG18AC_OTP,
> > > +  feature_buf);
> > > +}
> > > +
> > > +static int macronix_30lfxg18ac_otp_disable(struct nand_chip *nand)
> > > +{
> > > + u8 feature_buf[ONFI_SUBFEATURE_PARAM_LEN] = { 0 };
> > > +
> > > + return nand_set_features(nand, ONFI_FEATURE_ADDR_30LFXG18AC_OTP,
> > > +  feature_buf);
> > > +}
> > > +
> > > +static int __macronix_30lfxg18ac_rw_otp(struct mtd_info *mtd,
> > > + loff_t offs_in_flash,
> > > + size_t len, size_t *retlen,
> > > + u_char *buf, bool write)
> > > +{
> > > + struct nand_chip *nand;
> > > + size_t bytes_handled;
> > > + off_t 

Re: [PATCH 1/7] drm/panel: sitronix-st7789v: Prevent core spi warnings

2023-06-13 Thread Miquel Raynal
Hi Michael,

michael.rie...@wolfvision.net wrote on Tue, 13 Jun 2023 08:15:26 +0200:

> Hi Miquel,
> 
> On 6/9/23 16:59, Miquel Raynal wrote:
> > The spi core warns us about using an of_device_id table without a  
> 
> s/spi/SPI ?

Actually both are accepted in the kernel, IIRC it depends whether you
refer to the name of the bus or the Linux subsystem. Same for picking
"a" vs "an" before "spi/SPI". An attempt to use a unique formatting was
actually canceled because both are used equivalently and I believe even
the spi maintainer and the spi-nor maintainer kind of disagreed on the
default :)

> > spi_device_id table aside for module utilities in orter to not rely on  
> 
> s/in orter to/in order to ?

Yes, sorry for this typo as well as the two others you rightly pointed
out in another patch. I'll fix them.

> > OF modaliases. Just add this table using the device name without the
> > vendor prefix (as it is usually done).
> >
> > Signed-off-by: Miquel Raynal 
> > ---
> >  drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c  
> b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > index bbc4569cbcdc..c67b9adb157f 100644
> > --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > @@ -400,9 +400,16 @@ static const struct of_device_id  
> st7789v_of_match[] = {
> >  };
> >  MODULE_DEVICE_TABLE(of, st7789v_of_match);
> >
> > +static const struct spi_device_id st7789v_ids[] = {
> > +   { "st7789v", },
> > +   { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(spi, st7789v_ids);
> > +
> >  static struct spi_driver st7789v_driver = {
> > .probe = st7789v_probe,
> > .remove = st7789v_remove,
> > +   .id_table = st7789v_ids,
> > .driver = {
> > .name = "st7789v",
> > .of_match_table = st7789v_of_match,  
> 
> May I point to you Sebastian Reichel's series that features a partial
> overlap with your work? [0]

Woow. That driver has been untouched for years and now two
contributions at the same time. Sebastian, what is the current state of
your series? Shall I base my work on top of yours? Or is it still too
premature and we shall instead try to merge both and contribute a new
version of the series bringing support for the two panels?

> For instance, the patch at hand is comparable to [1].
> 
> Cc: Sebastian to keep him in the loop.

Thanks a lot for pointing this out.

> Best regards,
> Michael
> 
> [0]
> https://lore.kernel.org/dri-devel/20230422205012.2464933-1-...@kernel.org/
> [1]
> https://lore.kernel.org/dri-devel/20230422205012.2464933-4-...@kernel.org/


Thanks,
Miquèl


Re: [PATCH v4] mtd: rawnand: macronix: OTP access for MX30LFxG18AC

2023-06-12 Thread Miquel Raynal
Hello,

avkras...@sberdevices.ru wrote on Tue, 23 May 2023 13:16:34 +0300:

> This adds support for OTP area access on MX30LFxG18AC chip series.

Jaime, any feedback on this? Will you test it?

How are we supposed to test the OTP is locked? I see this is still an
open point.

> 
> Signed-off-by: Arseniy Krasnov 
> ---
>   v1 -> v2:
>   * Add slab.h include due to kernel test robot error.
>   v2 -> v3:
>   * Use 'uint64_t' as input argument for 'do_div()' instead
> of 'unsigned long' due to kernel test robot error.
>   v3 -> v4:
>   * Use 'dev_err()' instead of 'WARN()'.
>   * Call 'match_string()' before checking 'supports_set_get_features'
> in 'macronix_nand_setup_otp().
>   * Use 'u8' instead of 'uint8_t' as ./checkpatch.pl wants.
> 
>  drivers/mtd/nand/raw/nand_macronix.c | 216 +++
>  1 file changed, 216 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/nand_macronix.c 
> b/drivers/mtd/nand/raw/nand_macronix.c
> index 1472f925f386..be1ffa93bebb 100644
> --- a/drivers/mtd/nand/raw/nand_macronix.c
> +++ b/drivers/mtd/nand/raw/nand_macronix.c
> @@ -6,6 +6,7 @@
>   * Author: Boris Brezillon 
>   */
>  
> +#include 
>  #include "linux/delay.h"
>  #include "internals.h"
>  
> @@ -31,6 +32,20 @@
>  
>  #define MXIC_CMD_POWER_DOWN 0xB9
>  
> +#define ONFI_FEATURE_ADDR_30LFXG18AC_OTP 0x90
> +#define MACRONIX_30LFXG18AC_OTP_START_PAGE   0
> +#define MACRONIX_30LFXG18AC_OTP_PAGES30
> +#define MACRONIX_30LFXG18AC_OTP_PAGE_SIZE2112
> +#define MACRONIX_30LFXG18AC_OTP_START_BYTE   \
> + (MACRONIX_30LFXG18AC_OTP_START_PAGE *   \
> +  MACRONIX_30LFXG18AC_OTP_PAGE_SIZE)
> +#define MACRONIX_30LFXG18AC_OTP_SIZE_BYTES   \
> + (MACRONIX_30LFXG18AC_OTP_PAGES *\
> +  MACRONIX_30LFXG18AC_OTP_PAGE_SIZE)
> +
> +#define MACRONIX_30LFXG18AC_OTP_EN   BIT(0)
> +#define MACRONIX_30LFXG18AC_OTP_LOCKED   BIT(1)
> +
>  struct nand_onfi_vendor_macronix {
>   u8 reserved;
>   u8 reliability_func;
> @@ -316,6 +331,206 @@ static void 
> macronix_nand_deep_power_down_support(struct nand_chip *chip)
>   chip->ops.resume = mxic_nand_resume;
>  }
>  
> +static int macronix_30lfxg18ac_get_otp_info(struct mtd_info *mtd, size_t len,
> + size_t *retlen,
> + struct otp_info *buf)
> +{
> + if (len < sizeof(*buf))
> + return -EINVAL;
> +
> + /* Don't know how to check that OTP is locked. */
> + buf->locked = 0;
> + buf->start = MACRONIX_30LFXG18AC_OTP_START_BYTE;
> + buf->length = MACRONIX_30LFXG18AC_OTP_SIZE_BYTES;
> +
> + *retlen = sizeof(*buf);
> +
> + return 0;
> +}
> +
> +static int macronix_30lfxg18ac_otp_enable(struct nand_chip *nand)
> +{
> + u8 feature_buf[ONFI_SUBFEATURE_PARAM_LEN] = { 0 };
> +
> + feature_buf[0] = MACRONIX_30LFXG18AC_OTP_EN;
> + return nand_set_features(nand, ONFI_FEATURE_ADDR_30LFXG18AC_OTP,
> +  feature_buf);
> +}
> +
> +static int macronix_30lfxg18ac_otp_disable(struct nand_chip *nand)
> +{
> + u8 feature_buf[ONFI_SUBFEATURE_PARAM_LEN] = { 0 };
> +
> + return nand_set_features(nand, ONFI_FEATURE_ADDR_30LFXG18AC_OTP,
> +  feature_buf);
> +}
> +
> +static int __macronix_30lfxg18ac_rw_otp(struct mtd_info *mtd,
> + loff_t offs_in_flash,
> + size_t len, size_t *retlen,
> + u_char *buf, bool write)
> +{
> + struct nand_chip *nand;
> + size_t bytes_handled;
> + off_t offs_in_page;
> + void *dma_buf;
> + u64 page;
> + int ret;
> +
> + /* 'nand_prog/read_page_op()' may use 'buf' as DMA buffer,
> +  * so allocate properly aligned memory for it. This is
> +  * needed because cross page accesses may lead to unaligned
> +  * buffer address for DMA.
> +  */
> + dma_buf = kmalloc(MACRONIX_30LFXG18AC_OTP_PAGE_SIZE, GFP_KERNEL);
> + if (!dma_buf)
> + return -ENOMEM;
> +
> + nand = mtd_to_nand(mtd);
> + nand_select_target(nand, 0);
> +
> + ret = macronix_30lfxg18ac_otp_enable(nand);
> + if (ret)
> + goto out_otp;
> +
> + page = offs_in_flash;
> + /* 'page' will be result of division. */
> + offs_in_page = do_div(page, MACRONIX_30LFXG18AC_OTP_PAGE_SIZE);
> + bytes_handled = 0;
> +
> + while (bytes_handled < len &&
> +page < MACRONIX_30LFXG18AC_OTP_PAGES) {
> + size_t bytes_to_handle;
> +
> + bytes_to_handle = min_t(size_t, len - bytes_handled,
> + MACRONIX_30LFXG18AC_OTP_PAGE_SIZE -
> + offs_in_page);
> +
> + if (write) {
> + memcpy(dma_buf, [bytes_handled], bytes_to_handle);
> + ret = nand_prog_page_op(nand, page, offs_in_page,
> +  

[PATCH v2 1/2] of: module: Export of_device_uevent()

2023-06-09 Thread Miquel Raynal
The content of of_device_uevent() is currently hardcoded in a driver
that can be compiled as a module. Nothing prevents of_device_uevent() to
be exported to modules, most of the other helpers in of/device.c
actually are. The reason why this helper was not exported is because it
has been so far only useful in drivers/base, which is built-in anyway.

With the idea of getting rid of the hardcoded implementation of
of_device_uevent() in other places in the kernel, let's export it to GPL
modules (very much like its cousins in the same file).

Signed-off-by: Miquel Raynal 
---
 drivers/of/device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 0f00f1b80708..90131de6d75b 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -312,6 +312,7 @@ void of_device_uevent(const struct device *dev, struct 
kobj_uevent_env *env)
}
mutex_unlock(_mutex);
 }
+EXPORT_SYMBOL_GPL(of_device_uevent);
 
 int of_device_uevent_modalias(const struct device *dev, struct kobj_uevent_env 
*env)
 {
-- 
2.34.1



[PATCH v2 2/2] gpu: host1x: Stop open-coding of_device_uevent()

2023-06-09 Thread Miquel Raynal
There is apparently no reasons to open-code of_device_uevent() besides:
- The helper receives a struct device while we want to use the of_node
  member of the struct device *parent*.
- of_device_uevent() could not be called by modules because of a missing
  EXPORT_SYMBOL*().

In practice, the former point is not very constraining, just calling
of_device_uevent(dev->parent, ...) would have made the trick.

The latter point is more an observation rather than a real blocking
point because nothing prevented of_uevent() (called by the inline
function of_device_uevent()) to be exported to modules. In practice,
this helper is now exported, so nothing prevent us from using
of_device_uevent() anymore.

Let's use the core helper directly instead of open-coding it.

Cc: Thierry Reding 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Mikko Perttunen 
Cc: Rob Herring 
Cc: Frank Rowand 
Signed-off-by: Miquel Raynal 

---

This patch depends on the changes performed earlier in the series under
the drivers/of/ folder.
---
 drivers/gpu/host1x/bus.c | 29 ++---
 1 file changed, 6 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
index 4d16a3396c4a..dae589b83be1 100644
--- a/drivers/gpu/host1x/bus.c
+++ b/drivers/gpu/host1x/bus.c
@@ -338,32 +338,15 @@ static int host1x_device_match(struct device *dev, struct 
device_driver *drv)
return strcmp(dev_name(dev), drv->name) == 0;
 }
 
+/*
+ * Note that this is really only needed for backwards compatibility
+ * with libdrm, which parses this information from sysfs and will
+ * fail if it can't find the OF_FULLNAME, specifically.
+ */
 static int host1x_device_uevent(const struct device *dev,
struct kobj_uevent_env *env)
 {
-   struct device_node *np = dev->parent->of_node;
-   unsigned int count = 0;
-   struct property *p;
-   const char *compat;
-
-   /*
-* This duplicates most of of_device_uevent(), but the latter cannot
-* be called from modules and operates on dev->of_node, which is not
-* available in this case.
-*
-* Note that this is really only needed for backwards compatibility
-* with libdrm, which parses this information from sysfs and will
-* fail if it can't find the OF_FULLNAME, specifically.
-*/
-   add_uevent_var(env, "OF_NAME=%pOFn", np);
-   add_uevent_var(env, "OF_FULLNAME=%pOF", np);
-
-   of_property_for_each_string(np, "compatible", p, compat) {
-   add_uevent_var(env, "OF_COMPATIBLE_%u=%s", count, compat);
-   count++;
-   }
-
-   add_uevent_var(env, "OF_COMPATIBLE_N=%u", count);
+   of_device_uevent((const struct device *)>parent, env);
 
return 0;
 }
-- 
2.34.1



[PATCH v2 0/2] Small of/device cleanup

2023-06-09 Thread Miquel Raynal
My previous attempt to slightly clean the OF core wrt device structures
was rather unsuccessful as the idea behind the discussed cleanup was
more impacting than what I thought, leading to most of the previous
series to be dropped. However, aside, two patches seemed actually
relevant, so here they are, alone.

Link: https://lore.kernel.org/all/20230608184903.ga3200973-r...@kernel.org/

Thanks,
Miquèl

Changes in v2:
* Dropped all the of_device.h/of_module.h changes
* Directly used of_device_uevent() from the host1x driver

Miquel Raynal (2):
  of: module: Export of_device_uevent()
  gpu: host1x: Stop open-coding of_device_uevent()

 drivers/gpu/host1x/bus.c | 29 ++---
 drivers/of/device.c  |  1 +
 2 files changed, 7 insertions(+), 23 deletions(-)

-- 
2.34.1



[PATCH] drm: atmel-hlcdc: Support inverting the pixel clock polarity

2023-06-09 Thread Miquel Raynal
On the SoC host controller, the pixel clock can be:
* standard: data is launched on the rising edge
* inverted: data is launched on the falling edge

Some panels may need the inverted option to be used so let's support
this DRM flag.

Signed-off-by: Miquel Raynal 
---

Hello, this change was tested on a Sama5d36 based custom board with a
panel not behaving correctly if the pixel clock was not inverted.
Cheers, Miquèl

 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 25 +++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 58184cd6ab0b..cc5cf4c2faf7 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -68,7 +68,11 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c);
struct regmap *regmap = crtc->dc->hlcdc->regmap;
struct drm_display_mode *adj = >state->adjusted_mode;
+   struct drm_encoder *encoder = NULL, *en_iter;
+   struct drm_connector *connector = NULL;
struct atmel_hlcdc_crtc_state *state;
+   struct drm_device *ddev = c->dev;
+   struct drm_connector_list_iter iter;
unsigned long mode_rate;
struct videomode vm;
unsigned long prate;
@@ -76,6 +80,23 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
unsigned int cfg = 0;
int div, ret;
 
+   /* get encoder from crtc */
+   drm_for_each_encoder(en_iter, ddev) {
+   if (en_iter->crtc == c) {
+   encoder = en_iter;
+   break;
+   }
+   }
+
+   if (encoder) {
+   /* Get the connector from encoder */
+   drm_connector_list_iter_begin(ddev, );
+   drm_for_each_connector_iter(connector, )
+   if (connector->encoder == encoder)
+   break;
+   drm_connector_list_iter_end();
+   }
+
ret = clk_prepare_enable(crtc->dc->hlcdc->sys_clk);
if (ret)
return;
@@ -134,6 +155,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
 
cfg |= ATMEL_HLCDC_CLKDIV(div);
 
+   if (connector &&
+   connector->display_info.bus_flags & 
DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
+   cfg |= ATMEL_HLCDC_CLKPOL;
+
regmap_update_bits(regmap, ATMEL_HLCDC_CFG(0), mask, cfg);
 
state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
-- 
2.34.1



[PATCH 7/7] drm/panel: sitronix-st7789v: Check display ID

2023-06-09 Thread Miquel Raynal
A very basic debugging rule when a device is connected for the first
time is to access a read-only register which contains known data in
order to ensure the communication protocol is properly working. This
driver lacked any read helper which is often a critical peace for
fastening bring-ups.

Add a read helper and use it to verify the communication with the panel
is working as soon as possible in order to fail early if this is not the
case.

Signed-off-by: Miquel Raynal 
---
 .../gpu/drm/panel/panel-sitronix-st7789v.c| 78 +++
 1 file changed, 78 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index 7de192a3a8aa..fb21d52a7a63 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -113,6 +113,9 @@
return val; \
} while (0)
 
+#define ST7789V_IDS { 0x85, 0x85, 0x52 }
+#define ST7789V_IDS_SIZE 3
+
 struct st7789v_panel_info {
const struct drm_display_mode *display_mode;
u16 width_mm;
@@ -165,6 +168,77 @@ static int st7789v_write_data(struct st7789v *ctx, u8 cmd)
return st7789v_spi_write(ctx, ST7789V_DATA, cmd);
 }
 
+static int st7789v_read_data(struct st7789v *ctx, u8 cmd, u8 *buf,
+unsigned int len)
+{
+   struct spi_transfer xfer[2] = { };
+   struct spi_message msg;
+   u16 txbuf = ((ST7789V_COMMAND & 1) << 8) | cmd;
+   u16 rxbuf[4] = {};
+   u8 bit9 = 0;
+   int ret, i;
+
+   switch (len) {
+   case 1:
+   case 3:
+   case 4:
+   break;
+   default:
+   return -EOPNOTSUPP;
+   }
+
+   spi_message_init();
+
+   xfer[0].tx_buf = 
+   xfer[0].len = sizeof(txbuf);
+   spi_message_add_tail([0], );
+
+   xfer[1].rx_buf = rxbuf;
+   xfer[1].len = len * 2;
+   spi_message_add_tail([1], );
+
+   ret = spi_sync(ctx->spi, );
+   if (ret)
+   return ret;
+
+   for (i = 0; i < len; i++) {
+   buf[i] = rxbuf[i] >> i | (bit9 << (9 - i));
+   if (i)
+   bit9 = rxbuf[i] & GENMASK(i - 1, 0);
+   }
+
+   return 0;
+}
+
+static int st7789v_check_id(struct drm_panel *panel)
+{
+   const u8 st7789v_ids[ST7789V_IDS_SIZE] = ST7789V_IDS;
+   struct st7789v *ctx = panel_to_st7789v(panel);
+   bool invalid_ids = false;
+   int ret, i;
+   u8 ids[3];
+
+   ret = st7789v_read_data(ctx, MIPI_DCS_GET_DISPLAY_ID, ids, 
ST7789V_IDS_SIZE);
+   if (ret) {
+   dev_err(panel->dev, "Failed to read IDs\n");
+   return ret;
+   }
+
+   for (i = 0; i < ST7789V_IDS_SIZE; i++) {
+   if (ids[i] != st7789v_ids[i]) {
+   invalid_ids = true;
+   break;
+   }
+   }
+
+   if (invalid_ids) {
+   dev_err(panel->dev, "Unrecognized panel IDs");
+   return -EIO;
+   }
+
+   return 0;
+}
+
 static const struct drm_display_mode default_mode = {
.clock = 7000,
.hdisplay = 240,
@@ -237,6 +311,10 @@ static int st7789v_prepare(struct drm_panel *panel)
gpiod_set_value(ctx->reset, 0);
msleep(120);
 
+   ret = st7789v_check_id(panel);
+   if (ret)
+   return ret;
+
ST7789V_TEST(ret, st7789v_write_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE));
 
/* We need to wait 120ms after a sleep out command */
-- 
2.34.1



[PATCH 2/7] drm/panel: sitronix-st7789v: Use 9 bits per spi word by default

2023-06-09 Thread Miquel Raynal
The Sitronix controller expects 9-bit words, provide this as default at
probe time rather than specifying this in each and every access.

Signed-off-by: Miquel Raynal 
---
 drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index c67b9adb157f..e9ca7ebb458a 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -138,7 +138,6 @@ static int st7789v_spi_write(struct st7789v *ctx, enum 
st7789v_prefix prefix,
spi_message_init();
 
xfer.tx_buf = 
-   xfer.bits_per_word = 9;
xfer.len = sizeof(txbuf);
 
spi_message_add_tail(, );
@@ -365,6 +364,13 @@ static int st7789v_probe(struct spi_device *spi)
spi_set_drvdata(spi, ctx);
ctx->spi = spi;
 
+   spi->bits_per_word = 9;
+   ret = spi_setup(spi);
+   if (ret < 0) {
+   dev_err(>dev, "spi_setup failed: %d\n", ret);
+   return ret;
+   }
+
drm_panel_init(>panel, >dev, _drm_funcs,
   DRM_MODE_CONNECTOR_DPI);
 
-- 
2.34.1



[PATCH 3/7] drm/panel: sitronix-st7789v: Specify the expected bus format

2023-06-09 Thread Miquel Raynal
The LCD controller supports RGB444, RGB565 and RGB888. The value that is
written in the COLMOD register indicates using RGB888, so let's clearly
specify the in-use bus format.

Signed-off-by: Miquel Raynal 
---
 drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index e9ca7ebb458a..0abb45bea18d 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -170,6 +171,7 @@ static int st7789v_get_modes(struct drm_panel *panel,
 struct drm_connector *connector)
 {
struct drm_display_mode *mode;
+   u32 bus_format = MEDIA_BUS_FMT_RGB666_1X18;
 
mode = drm_mode_duplicate(connector->dev, _mode);
if (!mode) {
@@ -186,6 +188,8 @@ static int st7789v_get_modes(struct drm_panel *panel,
 
connector->display_info.width_mm = 61;
connector->display_info.height_mm = 103;
+   drm_display_info_set_bus_formats(>display_info,
+_format, 1);
 
return 1;
 }
-- 
2.34.1



[PATCH 5/7] dt-bindings: display: st7789v: Add the edt, et028013dma panel compatible

2023-06-09 Thread Miquel Raynal
The ST7789V LCD controller is also embedded in the ET028013DMA panel. In
fact, "sitronix,st7789v" might not be totally relevant alone as most of
the time -if not all- the LCD controller will always be packaged into a
display with its own physical properties.

Let's keep "sitronix,st7789v" valid alone for backward compatibility,
but we should definitely provide two compatibles to fully describe such
panel, so let's expect to have both when describing a panel such as the
EDT ET028013DMA.

Signed-off-by: Miquel Raynal 
---
 .../bindings/display/panel/sitronix,st7789v.yaml   | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git 
a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml 
b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
index d984b59daa4a..d4c8af9a973d 100644
--- a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
+++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml
@@ -15,7 +15,12 @@ allOf:
 
 properties:
   compatible:
-const: sitronix,st7789v
+oneOf:
+  - items:
+  - enum:
+  - edt,et028013dma
+  - const: sitronix,st7789v
+  - const: sitronix,st7789v
 
   reg: true
   reset-gpios: true
-- 
2.34.1



[PATCH 6/7] drm/panel: sitronix-st7789v: Add EDT ET028013DMA panel support

2023-06-09 Thread Miquel Raynal
This panel from Emerging Display Technologies Corporation features an
ST7789V2 panel inside which is almost identical to what the Sitronix
panel driver supports.

In practice, the module physical size is specific, and experiments show
that the display will malfunction if any of the following situation
occurs:
* Pixel clock is above 3MHz
* Pixel clock is not inverted
I could not properly identify the reasons behind these failures, scope
captures show valid input signals.

Signed-off-by: Miquel Raynal 
---
 .../gpu/drm/panel/panel-sitronix-st7789v.c| 34 +--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index 212bccc31804..7de192a3a8aa 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -30,7 +30,8 @@
 #define ST7789V_RGBCTRL_RCM(n) (((n) & 3) << 5)
 #define ST7789V_RGBCTRL_VSYNC_HIGH BIT(3)
 #define ST7789V_RGBCTRL_HSYNC_HIGH BIT(2)
-#define ST7789V_RGBCTRL_PCLK_HIGH  BIT(1)
+#define ST7789V_RGBCTRL_PCLK_FALLING   BIT(1)
+#define ST7789V_RGBCTRL_PCLK_RISING0
 #define ST7789V_RGBCTRL_VBP(n) ((n) & 0x7f)
 #define ST7789V_RGBCTRL_HBP(n) ((n) & 0x1f)
 
@@ -117,6 +118,7 @@ struct st7789v_panel_info {
u16 width_mm;
u16 height_mm;
u32 bus_format;
+   u32 bus_flags;
 };
 
 struct st7789v {
@@ -175,6 +177,18 @@ static const struct drm_display_mode default_mode = {
.vtotal = 320 + 8 + 4 + 4,
 };
 
+static const struct drm_display_mode slow_mode = {
+   .clock = 3000,
+   .hdisplay = 240,
+   .hsync_start = 240 + 38,
+   .hsync_end = 240 + 38 + 10,
+   .htotal = 240 + 38 + 10 + 10,
+   .vdisplay = 320,
+   .vsync_start = 320 + 8,
+   .vsync_end = 320 + 8 + 4,
+   .vtotal = 320 + 8 + 4 + 4,
+};
+
 static int st7789v_get_modes(struct drm_panel *panel,
 struct drm_connector *connector)
 {
@@ -197,6 +211,7 @@ static int st7789v_get_modes(struct drm_panel *panel,
 
connector->display_info.width_mm = panel_info->width_mm;
connector->display_info.height_mm = panel_info->height_mm;
+   connector->display_info.bus_flags = panel_info->bus_flags;
drm_display_info_set_bus_formats(>display_info,
 _info->bus_format, 1);
 
@@ -206,8 +221,13 @@ static int st7789v_get_modes(struct drm_panel *panel,
 static int st7789v_prepare(struct drm_panel *panel)
 {
struct st7789v *ctx = panel_to_st7789v(panel);
+   const struct st7789v_panel_info *panel_info = ctx->panel_info;
+   u8 pck = ST7789V_RGBCTRL_PCLK_FALLING;
int ret;
 
+   if (panel_info->bus_flags & DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE)
+   pck = ST7789V_RGBCTRL_PCLK_RISING;
+
ret = regulator_enable(ctx->power);
if (ret)
return ret;
@@ -321,7 +341,7 @@ static int st7789v_prepare(struct drm_panel *panel)
 ST7789V_RGBCTRL_RCM(2) |
 ST7789V_RGBCTRL_VSYNC_HIGH |
 ST7789V_RGBCTRL_HSYNC_HIGH |
-ST7789V_RGBCTRL_PCLK_HIGH));
+pck));
ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RGBCTRL_VBP(8)));
ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RGBCTRL_HBP(20)));
 
@@ -422,14 +442,24 @@ static const struct st7789v_panel_info st7789v_info = {
.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
 };
 
+static const struct st7789v_panel_info et028013dma_info = {
+   .display_mode = _mode,
+   .width_mm = 43,
+   .height_mm = 58,
+   .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
+   .bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
+};
+
 static const struct of_device_id st7789v_of_match[] = {
{ .compatible = "sitronix,st7789v", .data = _info },
+   { .compatible = "edt,et028013dma", .data = _info },
{ }
 };
 MODULE_DEVICE_TABLE(of, st7789v_of_match);
 
 static const struct spi_device_id st7789v_ids[] = {
{ "st7789v", },
+   { "et028013dma", },
{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(spi, st7789v_ids);
-- 
2.34.1



[PATCH 1/7] drm/panel: sitronix-st7789v: Prevent core spi warnings

2023-06-09 Thread Miquel Raynal
The spi core warns us about using an of_device_id table without a
spi_device_id table aside for module utilities in orter to not rely on
OF modaliases. Just add this table using the device name without the
vendor prefix (as it is usually done).

Signed-off-by: Miquel Raynal 
---
 drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index bbc4569cbcdc..c67b9adb157f 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -400,9 +400,16 @@ static const struct of_device_id st7789v_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, st7789v_of_match);
 
+static const struct spi_device_id st7789v_ids[] = {
+   { "st7789v", },
+   { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(spi, st7789v_ids);
+
 static struct spi_driver st7789v_driver = {
.probe = st7789v_probe,
.remove = st7789v_remove,
+   .id_table = st7789v_ids,
.driver = {
.name = "st7789v",
.of_match_table = st7789v_of_match,
-- 
2.34.1



[PATCH 4/7] drm/panel: sitronix-st7789v: Use platform data

2023-06-09 Thread Miquel Raynal
The Sitronix ST7789V LCD controller is actually packaged in a number of
different panels with slightly different properties. Before introducing
the support for another pannel using this same LCD controller, let's
move all the panel-specific information into a dedicated structure that
is available as platform data.

There is no functional change.

Signed-off-by: Miquel Raynal 
---
 .../gpu/drm/panel/panel-sitronix-st7789v.c| 30 +++
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index 0abb45bea18d..212bccc31804 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -112,11 +112,19 @@
return val; \
} while (0)
 
+struct st7789v_panel_info {
+   const struct drm_display_mode *display_mode;
+   u16 width_mm;
+   u16 height_mm;
+   u32 bus_format;
+};
+
 struct st7789v {
struct drm_panel panel;
struct spi_device *spi;
struct gpio_desc *reset;
struct regulator *power;
+   const struct st7789v_panel_info *panel_info;
 };
 
 enum st7789v_prefix {
@@ -170,10 +178,11 @@ static const struct drm_display_mode default_mode = {
 static int st7789v_get_modes(struct drm_panel *panel,
 struct drm_connector *connector)
 {
+   struct st7789v *ctx = panel_to_st7789v(panel);
+   const struct st7789v_panel_info *panel_info = ctx->panel_info;
struct drm_display_mode *mode;
-   u32 bus_format = MEDIA_BUS_FMT_RGB666_1X18;
 
-   mode = drm_mode_duplicate(connector->dev, _mode);
+   mode = drm_mode_duplicate(connector->dev, panel_info->display_mode);
if (!mode) {
dev_err(panel->dev, "failed to add mode %ux%ux@%u\n",
default_mode.hdisplay, default_mode.vdisplay,
@@ -186,10 +195,10 @@ static int st7789v_get_modes(struct drm_panel *panel,
mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
drm_mode_probed_add(connector, mode);
 
-   connector->display_info.width_mm = 61;
-   connector->display_info.height_mm = 103;
+   connector->display_info.width_mm = panel_info->width_mm;
+   connector->display_info.height_mm = panel_info->height_mm;
drm_display_info_set_bus_formats(>display_info,
-_format, 1);
+_info->bus_format, 1);
 
return 1;
 }
@@ -365,6 +374,8 @@ static int st7789v_probe(struct spi_device *spi)
if (!ctx)
return -ENOMEM;
 
+   ctx->panel_info = device_get_match_data(>dev);
+
spi_set_drvdata(spi, ctx);
ctx->spi = spi;
 
@@ -404,8 +415,15 @@ static void st7789v_remove(struct spi_device *spi)
drm_panel_remove(>panel);
 }
 
+static const struct st7789v_panel_info st7789v_info = {
+   .display_mode = _mode,
+   .width_mm = 64,
+   .height_mm = 103,
+   .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
+};
+
 static const struct of_device_id st7789v_of_match[] = {
-   { .compatible = "sitronix,st7789v" },
+   { .compatible = "sitronix,st7789v", .data = _info },
{ }
 };
 MODULE_DEVICE_TABLE(of, st7789v_of_match);
-- 
2.34.1



[PATCH 0/7] drm/panel: sitronix-st7789v: Support ET028013DMA panel

2023-06-09 Thread Miquel Raynal
Hello,

The aim of this series is to add support for the EDT ET028013DMA
panel. This panel features a Sitronix ST7789V2 LCD controller, which is
already supported mainline (or very close to the ST7789V for which
Maxime added support years ago).

The EDT panel is slightly different on the geometry and appears not to
support refresh rates higher than 30fps (above, glitches are visible,
despite the incoming signals being rather clean). While I was working on
this panel, I found quite inconvenient to not be able to read anything
back as it is a great tool for debugging purposes. So the last patch
actually adds a read helper and uses it to perform a sanity check at
probe time by verifying the Sitronix controller IDs. If deemed
irrelevant, this patch may be discarded.

Thanks,
Miquèl

Miquel Raynal (7):
  drm/panel: sitronix-st7789v: Prevent core spi warnings
  drm/panel: sitronix-st7789v: Use 9 bits per spi word by default
  drm/panel: sitronix-st7789v: Specify the expected bus format
  drm/panel: sitronix-st7789v: Use platform data
  dt-bindings: display: st7789v: Add the edt,et028013dma panel
compatible
  drm/panel: sitronix-st7789v: Add EDT ET028013DMA panel support
  drm/panel: sitronix-st7789v: Check display ID

 .../display/panel/sitronix,st7789v.yaml   |   7 +-
 .../gpu/drm/panel/panel-sitronix-st7789v.c| 157 +-
 2 files changed, 156 insertions(+), 8 deletions(-)

-- 
2.34.1



Re: [RESEND PATCH v3] mtd: rawnand: macronix: OTP access for MX30LFxG18AC

2023-05-22 Thread Miquel Raynal
Hi Arseniy,

avkras...@sberdevices.ru wrote on Mon, 15 May 2023 12:49:50 +0300:

> Hello @Miquel!
> 
> Sorry, but who could review this patch? :) IIUC this logic is very hw 
> specific and we need
> someone who knows it well? I tested this patch on our devices (with already 
> known Meson NAND
> controller).

+ Jaime, who might test

> 
> Thanks, Arseniy
> 
> On 11.05.2023 21:21, Arseniy Krasnov wrote:
> > Cc: Mason Yang  and Boris Brezillon 
> > 
> > 
> > On 11.05.2023 18:21, Arseniy Krasnov wrote:  
> >> This adds support for OTP area access on MX30LFxG18AC chip series.
> >>
> >> Changelog:
> >>   v1 -> v2:
> >>   * Add slab.h include due to kernel test robot error.
> >>   v2 -> v3:
> >>   * Use 'uint64_t' as input argument for 'do_div()' instead
> >> of 'unsigned long' due to kernel test robot error.
> >>
> >> Signed-off-by: Arseniy Krasnov 
> >> ---
> >>  drivers/mtd/nand/raw/nand_macronix.c | 213 +++
> >>  1 file changed, 213 insertions(+)
> >>
> >> diff --git a/drivers/mtd/nand/raw/nand_macronix.c 
> >> b/drivers/mtd/nand/raw/nand_macronix.c
> >> index 1472f925f386..2301f990678e 100644
> >> --- a/drivers/mtd/nand/raw/nand_macronix.c
> >> +++ b/drivers/mtd/nand/raw/nand_macronix.c
> >> @@ -6,6 +6,7 @@
> >>   * Author: Boris Brezillon 
> >>   */
> >>  
> >> +#include 
> >>  #include "linux/delay.h"
> >>  #include "internals.h"
> >>  
> >> @@ -31,6 +32,20 @@
> >>  
> >>  #define MXIC_CMD_POWER_DOWN 0xB9
> >>  
> >> +#define ONFI_FEATURE_ADDR_30LFXG18AC_OTP  0x90
> >> +#define MACRONIX_30LFXG18AC_OTP_START_PAGE0
> >> +#define MACRONIX_30LFXG18AC_OTP_PAGES 30
> >> +#define MACRONIX_30LFXG18AC_OTP_PAGE_SIZE 2112
> >> +#define MACRONIX_30LFXG18AC_OTP_START_BYTE\
> >> +  (MACRONIX_30LFXG18AC_OTP_START_PAGE *   \
> >> +   MACRONIX_30LFXG18AC_OTP_PAGE_SIZE)
> >> +#define MACRONIX_30LFXG18AC_OTP_SIZE_BYTES\
> >> +  (MACRONIX_30LFXG18AC_OTP_PAGES *\
> >> +   MACRONIX_30LFXG18AC_OTP_PAGE_SIZE)
> >> +
> >> +#define MACRONIX_30LFXG18AC_OTP_ENBIT(0)
> >> +#define MACRONIX_30LFXG18AC_OTP_LOCKEDBIT(1)
> >> +
> >>  struct nand_onfi_vendor_macronix {
> >>u8 reserved;
> >>u8 reliability_func;
> >> @@ -316,6 +331,203 @@ static void 
> >> macronix_nand_deep_power_down_support(struct nand_chip *chip)
> >>chip->ops.resume = mxic_nand_resume;
> >>  }
> >>  
> >> +static int macronix_30lfxg18ac_get_otp_info(struct mtd_info *mtd, size_t 
> >> len,
> >> +  size_t *retlen,
> >> +  struct otp_info *buf)
> >> +{
> >> +  if (len < sizeof(*buf))
> >> +  return -EINVAL;
> >> +
> >> +  /* Don't know how to check that OTP is locked. */

Jaime, any idea?

> >> +  buf->locked = 0;
> >> +  buf->start = MACRONIX_30LFXG18AC_OTP_START_BYTE;
> >> +  buf->length = MACRONIX_30LFXG18AC_OTP_SIZE_BYTES;
> >> +
> >> +  *retlen = sizeof(*buf);
> >> +
> >> +  return 0;
> >> +}
> >> +
> >> +static int macronix_30lfxg18ac_otp_enable(struct nand_chip *nand)
> >> +{
> >> +  uint8_t feature_buf[ONFI_SUBFEATURE_PARAM_LEN] = { 0 };
> >> +
> >> +  feature_buf[0] = MACRONIX_30LFXG18AC_OTP_EN;
> >> +  return nand_set_features(nand, ONFI_FEATURE_ADDR_30LFXG18AC_OTP,
> >> +   feature_buf);
> >> +}
> >> +
> >> +static int macronix_30lfxg18ac_otp_disable(struct nand_chip *nand)
> >> +{
> >> +  uint8_t feature_buf[ONFI_SUBFEATURE_PARAM_LEN] = { 0 };
> >> +
> >> +  return nand_set_features(nand, ONFI_FEATURE_ADDR_30LFXG18AC_OTP,
> >> +   feature_buf);
> >> +}
> >> +
> >> +static int __macronix_30lfxg18ac_rw_otp(struct mtd_info *mtd,
> >> +  loff_t offs_in_flash,
> >> +  size_t len, size_t *retlen,
> >> +  u_char *buf, bool write)
> >> +{
> >> +  struct nand_chip *nand;
> >> +  size_t bytes_handled;
> >> +  off_t offs_in_page;
> >> +  uint64_t page;
> >> +  void *dma_buf;
> >> +  int ret;
> >> +
> >> +  /* 'nand_prog/read_page_op()' may use 'buf' as DMA buffer,
> >> +   * so allocate properly aligned memory for it. This is
> >> +   * needed because cross page accesses may lead to unaligned
> >> +   * buffer address for DMA.
> >> +   */
> >> +  dma_buf = kmalloc(MACRONIX_30LFXG18AC_OTP_PAGE_SIZE, GFP_KERNEL);
> >> +  if (!dma_buf)
> >> +  return -ENOMEM;
> >> +
> >> +  nand = mtd_to_nand(mtd);
> >> +  nand_select_target(nand, 0);
> >> +
> >> +  ret = macronix_30lfxg18ac_otp_enable(nand);
> >> +  if (ret)
> >> +  goto out_otp;
> >> +
> >> +  page = offs_in_flash;
> >> +  /* 'page' will be result of division. */
> >> +  offs_in_page = do_div(page, MACRONIX_30LFXG18AC_OTP_PAGE_SIZE);
> >> +  bytes_handled = 0;
> >> +
> >> +  while (bytes_handled < len &&
> >> + page < MACRONIX_30LFXG18AC_OTP_PAGES) {
> >> +  size_t bytes_to_handle;
> >> +
> >> +  bytes_to_handle = min_t(size_t, len - bytes_handled,
> 

[PATCH 5/5] gpu: host1x: Stop open-coding of_device_uevent()

2023-05-10 Thread Miquel Raynal
There is apparently no reasons to open-code of_device_uevent() besides:
- The helper receives a struct device while we want to use the of_node
  member of the struct device *parent*.
- of_device_uevent() could not be called by modules because of a missing
  EXPORT_SYMBOL*().

In practice, the former point is not very constraining, just calling
of_device_uevent(dev->parent, ...) would have made the trick.

The latter point is more an observation rather than a real blocking
point because nothing prevented of_uevent() (called by the inline
function of_device_uevent()) to be exported to modules. In practice,
this helper is now exported, so nothing prevent us from using
of_device_uevent() anymore.

Let's use the core helper directly instead of open-coding it.

Cc: Thierry Reding 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Mikko Perttunen 
Cc: Rob Herring 
Cc: Frank Rowand 
Signed-off-by: Miquel Raynal 

---

This patch depends on the changes performed earlier in the series under
the drivers/of/ folder.
---
 drivers/gpu/host1x/bus.c | 31 ++-
 1 file changed, 6 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
index 4d16a3396c4a..6434a183fb72 100644
--- a/drivers/gpu/host1x/bus.c
+++ b/drivers/gpu/host1x/bus.c
@@ -338,34 +338,15 @@ static int host1x_device_match(struct device *dev, struct 
device_driver *drv)
return strcmp(dev_name(dev), drv->name) == 0;
 }
 
+/*
+ * Note that this is really only needed for backwards compatibility
+ * with libdrm, which parses this information from sysfs and will
+ * fail if it can't find the OF_FULLNAME, specifically.
+ */
 static int host1x_device_uevent(const struct device *dev,
struct kobj_uevent_env *env)
 {
-   struct device_node *np = dev->parent->of_node;
-   unsigned int count = 0;
-   struct property *p;
-   const char *compat;
-
-   /*
-* This duplicates most of of_device_uevent(), but the latter cannot
-* be called from modules and operates on dev->of_node, which is not
-* available in this case.
-*
-* Note that this is really only needed for backwards compatibility
-* with libdrm, which parses this information from sysfs and will
-* fail if it can't find the OF_FULLNAME, specifically.
-*/
-   add_uevent_var(env, "OF_NAME=%pOFn", np);
-   add_uevent_var(env, "OF_FULLNAME=%pOF", np);
-
-   of_property_for_each_string(np, "compatible", p, compat) {
-   add_uevent_var(env, "OF_COMPATIBLE_%u=%s", count, compat);
-   count++;
-   }
-
-   add_uevent_var(env, "OF_COMPATIBLE_N=%u", count);
-
-   return 0;
+   return of_device_uevent((const struct device *)>parent, env);
 }
 
 static int host1x_dma_configure(struct device *dev)
-- 
2.34.1



[PATCH 1/5] of: module: Mutate of_device_modalias() into two helpers

2023-05-10 Thread Miquel Raynal
Move the content of the helper providing printable modaliases in
module.c. Call this new function from an of_device.c inline helper.

There is no functional changes. However, as a side effect, we fix the
return value of the inline helper (in the !CONFIG_OF case) which should
be a ssize_t instead of int.

Signed-off-by: Miquel Raynal 
---
 drivers/of/device.c   | 25 -
 drivers/of/module.c   | 19 +++
 include/linux/of.h|  8 
 include/linux/of_device.h | 13 ++---
 4 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 0f00f1b80708..45ce83a8945f 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -246,31 +246,6 @@ const void *of_device_get_match_data(const struct device 
*dev)
 }
 EXPORT_SYMBOL(of_device_get_match_data);
 
-/**
- * of_device_modalias - Fill buffer with newline terminated modalias string
- * @dev:   Calling device
- * @str:   Modalias string
- * @len:   Size of @str
- */
-ssize_t of_device_modalias(struct device *dev, char *str, ssize_t len)
-{
-   ssize_t sl;
-
-   if (!dev || !dev->of_node || dev->of_node_reused)
-   return -ENODEV;
-
-   sl = of_modalias(dev->of_node, str, len - 2);
-   if (sl < 0)
-   return sl;
-   if (sl > len - 2)
-   return -ENOMEM;
-
-   str[sl++] = '\n';
-   str[sl] = 0;
-   return sl;
-}
-EXPORT_SYMBOL_GPL(of_device_modalias);
-
 /**
  * of_device_uevent - Display OF related uevent information
  * @dev:   Device to display the uevent information for
diff --git a/drivers/of/module.c b/drivers/of/module.c
index 0e8aa974f0f2..c05fb8ca575c 100644
--- a/drivers/of/module.c
+++ b/drivers/of/module.c
@@ -44,6 +44,25 @@ ssize_t of_modalias(const struct device_node *np, char *str, 
ssize_t len)
return tsize;
 }
 
+ssize_t of_printable_modalias(const struct device_node *np, char *str, ssize_t 
len)
+{
+   ssize_t sl;
+
+   if (!np)
+   return -ENODEV;
+
+   sl = of_modalias(np, str, len - 2);
+   if (sl < 0)
+   return sl;
+   if (sl > len - 2)
+   return -ENOMEM;
+
+   str[sl++] = '\n';
+   str[sl] = 0;
+   return sl;
+}
+EXPORT_SYMBOL_GPL(of_printable_modalias);
+
 int of_request_module(const struct device_node *np)
 {
char *str;
diff --git a/include/linux/of.h b/include/linux/of.h
index 6ecde0515677..dcdd9396ac37 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -385,6 +385,8 @@ extern int of_count_phandle_with_args(const struct 
device_node *np,
 
 /* module functions */
 extern ssize_t of_modalias(const struct device_node *np, char *str, ssize_t 
len);
+extern ssize_t of_printable_modalias(const struct device_node *np, char *str,
+ssize_t len);
 extern int of_request_module(const struct device_node *np);
 
 /* phandle iterator functions */
@@ -758,6 +760,12 @@ static inline ssize_t of_modalias(const struct device_node 
*np, char *str,
return -ENODEV;
 }
 
+static inline ssize_t of_printable_modalias(const struct device_node *np,
+   char *str, ssize_t len)
+{
+   return -ENODEV;
+}
+
 static inline int of_request_module(const struct device_node *np)
 {
return -ENODEV;
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index 2c7a3d4bc775..bca66f59814a 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -26,7 +26,14 @@ static inline int of_driver_match_device(struct device *dev,
return of_match_device(drv->of_match_table, dev) != NULL;
 }
 
-extern ssize_t of_device_modalias(struct device *dev, char *str, ssize_t len);
+static inline ssize_t of_device_modalias(struct device *dev, char *str,
+ssize_t len)
+{
+   if (!dev || !dev->of_node || dev->of_node_reused)
+   return -ENODEV;
+
+   return of_printable_modalias(dev->of_node, str, len);
+}
 
 extern void of_device_uevent(const struct device *dev, struct kobj_uevent_env 
*env);
 extern int of_device_uevent_modalias(const struct device *dev, struct 
kobj_uevent_env *env);
@@ -51,8 +58,8 @@ static inline int of_driver_match_device(struct device *dev,
 static inline void of_device_uevent(const struct device *dev,
struct kobj_uevent_env *env) { }
 
-static inline int of_device_modalias(struct device *dev,
-char *str, ssize_t len)
+static inline ssize_t of_device_modalias(struct device *dev,
+char *str, ssize_t len)
 {
return -ENODEV;
 }
-- 
2.34.1



[PATCH 3/5] of: module: Mutate of_device_uevent_modalias() into two helpers

2023-05-10 Thread Miquel Raynal
Let's move the logic of the former helper into module.c and use it from
an inline helper located under of_device.c. This way there is no change
for users while the logic gets moved to an OF-only file.

Signed-off-by: Miquel Raynal 
---
 drivers/of/device.c   | 23 ---
 drivers/of/module.c   | 21 +
 include/linux/of.h|  7 +++
 include/linux/of_device.h |  9 -
 4 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 5e538e1ed623..7909eefc650e 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -245,26 +245,3 @@ const void *of_device_get_match_data(const struct device 
*dev)
return match->data;
 }
 EXPORT_SYMBOL(of_device_get_match_data);
-
-int of_device_uevent_modalias(const struct device *dev, struct kobj_uevent_env 
*env)
-{
-   int sl;
-
-   if ((!dev) || (!dev->of_node) || dev->of_node_reused)
-   return -ENODEV;
-
-   /* Devicetree modalias is tricky, we add it in 2 steps */
-   if (add_uevent_var(env, "MODALIAS="))
-   return -ENOMEM;
-
-   sl = of_modalias(dev->of_node, >buf[env->buflen-1],
-sizeof(env->buf) - env->buflen);
-   if (sl < 0)
-   return sl;
-   if (sl >= (sizeof(env->buf) - env->buflen))
-   return -ENOMEM;
-   env->buflen += sl;
-
-   return 0;
-}
-EXPORT_SYMBOL_GPL(of_device_uevent_modalias);
diff --git a/drivers/of/module.c b/drivers/of/module.c
index c729675fdd04..874f3fb8220f 100644
--- a/drivers/of/module.c
+++ b/drivers/of/module.c
@@ -132,3 +132,24 @@ int of_uevent(struct device_node *np, struct 
kobj_uevent_env *env)
 
return 0;
 }
+
+int of_uevent_modalias(const struct device_node *np, struct kobj_uevent_env 
*env)
+{
+   int sl;
+
+   if (!np)
+   return -ENODEV;
+
+   /* Devicetree modalias is tricky, we add it in 2 steps */
+   if (add_uevent_var(env, "MODALIAS="))
+   return -ENOMEM;
+
+   sl = of_modalias(np, >buf[env->buflen-1],
+sizeof(env->buf) - env->buflen);
+   if (sl >= (sizeof(env->buf) - env->buflen))
+   return -ENOMEM;
+   env->buflen += sl;
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(of_uevent_modalias);
diff --git a/include/linux/of.h b/include/linux/of.h
index d99f33fc25d3..203bd2895d94 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -389,6 +389,7 @@ extern ssize_t of_printable_modalias(const struct 
device_node *np, char *str,
 ssize_t len);
 extern int of_request_module(const struct device_node *np);
 extern int of_uevent(struct device_node *np, struct kobj_uevent_env *env);
+extern int of_uevent_modalias(const struct device_node *np, struct 
kobj_uevent_env *env);
 
 /* phandle iterator functions */
 extern int of_phandle_iterator_init(struct of_phandle_iterator *it,
@@ -777,6 +778,12 @@ static inline int of_uevent(struct device_node *np, struct 
kobj_uevent_env *env)
return -ENODEV;
 }
 
+static inline int of_uevent_modalias(const struct device_node *np,
+struct kobj_uevent_env *env)
+{
+   return -ENODEV;
+}
+
 static inline int of_phandle_iterator_init(struct of_phandle_iterator *it,
   const struct device_node *np,
   const char *list_name,
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index af5ee78e0c05..5e428bcf3d63 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -44,7 +44,14 @@ static inline int of_device_uevent(const struct device *dev,
return of_uevent(dev->of_node, env);
 }
 
-extern int of_device_uevent_modalias(const struct device *dev, struct 
kobj_uevent_env *env);
+static inline int of_device_uevent_modalias(const struct device *dev,
+   struct kobj_uevent_env *env)
+{
+   if (!dev || !dev->of_node || dev->of_node_reused)
+   return -ENODEV;
+
+   return of_uevent_modalias(dev->of_node, env);
+}
 
 int of_dma_configure_id(struct device *dev,
 struct device_node *np,
-- 
2.34.1



[PATCH 0/5] of: More 'device' vs. 'module' cleanups

2023-05-10 Thread Miquel Raynal
Hello,

As part of a previous series, Rob suggested that keeping too much logic
in of/device.c was backward and would benefit from a gradual cleanup
with the hope some day to move the remaining helpers into inline
functions wrapping the proper of_*() methods.

Link: 
https://lore.kernel.org/lkml/cal_jsqje43qfyzhuucjsbappbtbyx05q7ffmptjpfz3dmz_...@mail.gmail.com/

A few of these "conversions" happened in the series I was originally
working on. At this time I actually wrote a few other changes,
completely unrelated to my original series, but still following Rob's
cleanup idea: here they are.

Link: 
https://lore.kernel.org/lkml/20230307165359.225361-1-miquel.ray...@bootlin.com/

The last step of this series is to actually remove a copy of one of
these helpers which I think is not needed. This drivers/gpu/ patch
depends on the previous changes.

Thanks, Miquèl

Miquel Raynal (5):
  of: module: Mutate of_device_modalias() into two helpers
  of: module: Mutate of_device_uevent() into two helpers
  of: module: Mutate of_device_uevent_modalias() into two helpers
  of: module: Export of_uevent()
  gpu: host1x: Stop open-coding of_device_uevent()

 drivers/gpu/host1x/bus.c  | 31 +++---
 drivers/of/device.c   | 90 ---
 drivers/of/module.c   | 82 +++
 include/linux/of.h| 21 +
 include/linux/of_device.h | 39 ++---
 5 files changed, 141 insertions(+), 122 deletions(-)

-- 
2.34.1



[PATCH 4/5] of: module: Export of_uevent()

2023-05-10 Thread Miquel Raynal
The content of of_uevent() is currently hardcoded in a driver that can
be compiled as a module. Nothing prevents of_uevent() to be exported to
modules, most of the other helpers in of_device.c actually are.The
reason why this helper was not exported is because it has been so far
only useful in drivers/base, which is built-in anyway.

With the idea of getting rid of the hardcoded implementation of
of_uevent() in other places in the kernel, let's export it to GPL
modules (very much like its cousins in the same file).

Signed-off-by: Miquel Raynal 
---
 drivers/of/module.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/of/module.c b/drivers/of/module.c
index 874f3fb8220f..8b402a716951 100644
--- a/drivers/of/module.c
+++ b/drivers/of/module.c
@@ -132,6 +132,7 @@ int of_uevent(struct device_node *np, struct 
kobj_uevent_env *env)
 
return 0;
 }
+EXPORT_SYMBOL_GPL(of_uevent);
 
 int of_uevent_modalias(const struct device_node *np, struct kobj_uevent_env 
*env)
 {
-- 
2.34.1



[PATCH 2/5] of: module: Mutate of_device_uevent() into two helpers

2023-05-10 Thread Miquel Raynal
Move the OF related logic inside of/module.c and use it from of_device.h
with an inline helper so there is no visible change from the users point
of view.

Signed-off-by: Miquel Raynal 
---
 drivers/of/device.c   | 42 ---
 drivers/of/module.c   | 41 ++
 include/linux/of.h|  6 ++
 include/linux/of_device.h | 17 +---
 4 files changed, 61 insertions(+), 45 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 45ce83a8945f..5e538e1ed623 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -246,48 +246,6 @@ const void *of_device_get_match_data(const struct device 
*dev)
 }
 EXPORT_SYMBOL(of_device_get_match_data);
 
-/**
- * of_device_uevent - Display OF related uevent information
- * @dev:   Device to display the uevent information for
- * @env:   Kernel object's userspace event reference to fill up
- */
-void of_device_uevent(const struct device *dev, struct kobj_uevent_env *env)
-{
-   const char *compat, *type;
-   struct alias_prop *app;
-   struct property *p;
-   int seen = 0;
-
-   if ((!dev) || (!dev->of_node))
-   return;
-
-   add_uevent_var(env, "OF_NAME=%pOFn", dev->of_node);
-   add_uevent_var(env, "OF_FULLNAME=%pOF", dev->of_node);
-   type = of_node_get_device_type(dev->of_node);
-   if (type)
-   add_uevent_var(env, "OF_TYPE=%s", type);
-
-   /* Since the compatible field can contain pretty much anything
-* it's not really legal to split it out with commas. We split it
-* up using a number of environment variables instead. */
-   of_property_for_each_string(dev->of_node, "compatible", p, compat) {
-   add_uevent_var(env, "OF_COMPATIBLE_%d=%s", seen, compat);
-   seen++;
-   }
-   add_uevent_var(env, "OF_COMPATIBLE_N=%d", seen);
-
-   seen = 0;
-   mutex_lock(_mutex);
-   list_for_each_entry(app, _lookup, link) {
-   if (dev->of_node == app->np) {
-   add_uevent_var(env, "OF_ALIAS_%d=%s", seen,
-  app->alias);
-   seen++;
-   }
-   }
-   mutex_unlock(_mutex);
-}
-
 int of_device_uevent_modalias(const struct device *dev, struct kobj_uevent_env 
*env)
 {
int sl;
diff --git a/drivers/of/module.c b/drivers/of/module.c
index c05fb8ca575c..c729675fdd04 100644
--- a/drivers/of/module.c
+++ b/drivers/of/module.c
@@ -8,6 +8,8 @@
 #include 
 #include 
 
+#include "of_private.h"
+
 ssize_t of_modalias(const struct device_node *np, char *str, ssize_t len)
 {
const char *compat;
@@ -91,3 +93,42 @@ int of_request_module(const struct device_node *np)
return ret;
 }
 EXPORT_SYMBOL_GPL(of_request_module);
+
+int of_uevent(struct device_node *np, struct kobj_uevent_env *env)
+{
+   const char *compat, *type;
+   struct alias_prop *app;
+   struct property *p;
+   int seen = 0;
+
+   if (!np)
+   return -ENODEV;
+
+   add_uevent_var(env, "OF_NAME=%pOFn", np);
+   add_uevent_var(env, "OF_FULLNAME=%pOF", np);
+   type = of_node_get_device_type(np);
+   if (type)
+   add_uevent_var(env, "OF_TYPE=%s", type);
+
+   /* Since the compatible field can contain pretty much anything
+* it's not really legal to split it out with commas. We split it
+* up using a number of environment variables instead. */
+   of_property_for_each_string(np, "compatible", p, compat) {
+   add_uevent_var(env, "OF_COMPATIBLE_%d=%s", seen, compat);
+   seen++;
+   }
+   add_uevent_var(env, "OF_COMPATIBLE_N=%d", seen);
+
+   seen = 0;
+   mutex_lock(_mutex);
+   list_for_each_entry(app, _lookup, link) {
+   if (np == app->np) {
+   add_uevent_var(env, "OF_ALIAS_%d=%s", seen,
+  app->alias);
+   seen++;
+   }
+   }
+   mutex_unlock(_mutex);
+
+   return 0;
+}
diff --git a/include/linux/of.h b/include/linux/of.h
index dcdd9396ac37..d99f33fc25d3 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -388,6 +388,7 @@ extern ssize_t of_modalias(const struct device_node *np, 
char *str, ssize_t len)
 extern ssize_t of_printable_modalias(const struct device_node *np, char *str,
 ssize_t len);
 extern int of_request_module(const struct device_node *np);
+extern int of_uevent(struct device_node *np, struct kobj_uevent_env *env);
 
 /* phandle iterator functions */
 extern int of_phandle_iterator_init(struct of_phandle_iterator *it,
@@ -771,6 +772,11 @@ static inline int of_request_module(const struct 

Re: [PATCH v3 28/65] clk: renesas: r9a06g032: Add a determine_rate hook

2023-04-11 Thread Miquel Raynal
Hi Geert & Maxime,

ge...@linux-m68k.org wrote on Tue, 11 Apr 2023 12:27:38 +0200:

> CC Gareth, Hervé, Miquel, Ralph
> 
> On Tue, Apr 4, 2023 at 2:44 PM Maxime Ripard  wrote:
> > The Renesas r9a06g032 bitselect clock implements a mux with a set_parent
> > hook, but doesn't provide a determine_rate implementation.
> >
> > This is a bit odd, since set_parent() is there to, as its name implies,
> > change the parent of a clock. However, the most likely candidate to
> > trigger that parent change is a call to clk_set_rate(), with
> > determine_rate() figuring out which parent is the best suited for a
> > given rate.
> >
> > The other trigger would be a call to clk_set_parent(), but it's far less
> > used, and it doesn't look like there's any obvious user for that clock.
> >
> > So, the set_parent hook is effectively unused, possibly because of an
> > oversight. However, it could also be an explicit decision by the
> > original author to avoid any reparenting but through an explicit call to
> > clk_set_parent().
> >
> > The latter case would be equivalent to setting the flag
> > CLK_SET_RATE_NO_REPARENT, together with setting our determine_rate hook
> > to __clk_mux_determine_rate(). Indeed, if no determine_rate
> > implementation is provided, clk_round_rate() (through
> > clk_core_round_rate_nolock()) will call itself on the parent if
> > CLK_SET_RATE_PARENT is set, and will not change the clock rate
> > otherwise. __clk_mux_determine_rate() has the exact same behavior when
> > CLK_SET_RATE_NO_REPARENT is set.
> >
> > And if it was an oversight, then we are at least explicit about our
> > behavior now and it can be further refined down the line.
> >
> > Signed-off-by: Maxime Ripard   
> 
> LGTM, so
> Reviewed-by: Geert Uytterhoeven 

I searched for 'possible callers', I didn't find any places
where this would be used on the consumer side. However, downstream,
there is a rzn1-clock-bitselect.c clock driver which states:

+ * This clock provider handles the case of the RZN1 where you have peripherals
+ * that have two potential clock source and two gates, one for each of the
+ * clock source - the used clock source (for all sub clocks) is selected by a
+ * single bit.
+ * That single bit affects all sub-clocks, and therefore needs to change the
+ * active gate (and turn the others off) and force a recalculation of the 
rates.

I don't know how much of this file has been upstreamed (under a
different form) but this might very well be related to the fact that
reparenting in some cases would be a major issue and thus needs to be
avoided unless done on purpose (guessing?).

Maybe Ralph can comment, but for what I understand,

Reviewed-by: Miquel Raynal 

> But I do not have the hardware.
> 
> > --- a/drivers/clk/renesas/r9a06g032-clocks.c
> > +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> > @@ -1121,6 +1121,7 @@ static int r9a06g032_clk_mux_set_parent(struct clk_hw 
> > *hw, u8 index)
> >  }
> >
> >  static const struct clk_ops clk_bitselect_ops = {
> > +   .determine_rate = __clk_mux_determine_rate,
> > .get_parent = r9a06g032_clk_mux_get_parent,
> > .set_parent = r9a06g032_clk_mux_set_parent,
> >  };
> > @@ -1145,7 +1146,7 @@ r9a06g032_register_bitsel(struct r9a06g032_priv 
> > *clocks,
> >
> > init.name = desc->name;
> > init.ops = _bitselect_ops;
> > -   init.flags = CLK_SET_RATE_PARENT;
> > +   init.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT;
> > init.parent_names = names;
> > init.num_parents = 2;
> >  
> 
> Gr{oetje,eeting}s,
> 
> Geert
> 

Thanks,
Miquèl


Re: [PATCH v1 03/11] mtd: rawnand: stm32_fmc2: switch to using devm_fwnode_gpiod_get()

2022-09-20 Thread Miquel Raynal
On Mon, 2022-09-05 at 06:30:55 UTC, Dmitry Torokhov wrote:
> I would like to stop exporting OF-specific devm_gpiod_get_from_of_node()
> so that gpiolib can be cleaned a bit, so let's switch to the generic
> fwnode property API.
> 
> Signed-off-by: Dmitry Torokhov 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
nand/next, thanks.

Miquel


Re: [PATCH 5/5] spi: make remove callback a void function

2022-01-26 Thread Miquel Raynal
Hi Uwe,

u.kleine-koe...@pengutronix.de wrote on Sun, 23 Jan 2022 18:52:01 +0100:

> The value returned by an spi driver's remove function is mostly ignored.
> (Only an error message is printed if the value is non-zero that the
> error is ignored.)
> 
> So change the prototype of the remove function to return no value. This
> way driver authors are not tempted to assume that passing an error to
> the upper layer is a good idea. All drivers are adapted accordingly.
> There is no intended change of behaviour, all callbacks were prepared to
> return 0 before.
> 
> Signed-off-by: Uwe Kleine-König 
> ---

[...]

>  drivers/mtd/devices/mchp23k256.c  |  4 +---
>  drivers/mtd/devices/mchp48l640.c  |  4 +---
>  drivers/mtd/devices/mtd_dataflash.c   |  4 +---
>  drivers/mtd/devices/sst25l.c      |  4 +---

For MTD devices:
Acked-by: Miquel Raynal 

Thanks,
Miquèl


Re: [PATCH v8 21/34] mtd: rawnand: tegra: Add runtime PM and OPP support

2021-08-18 Thread Miquel Raynal
Hi Dmitry,

Dmitry Osipenko  wrote on Tue, 17 Aug 2021 04:27:41
+0300:

> The NAND on Tegra belongs to the core power domain and we're going to
> enable GENPD support for the core domain. Now NAND must be resumed using
> runtime PM API in order to initialize the NAND power state. Add runtime PM
> and OPP support to the NAND driver.
> 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/mtd/nand/raw/tegra_nand.c | 62 +++
>  1 file changed, 54 insertions(+), 8 deletions(-)
> 

Acked-by: Miquel Raynal 

Thanks,
Miquèl


Re: [PATCH v3 07/23] mtd: spi-nor: hisi-sfc: Demote non-conformant kernel-doc

2020-11-20 Thread Miquel Raynal
On Mon, 2020-11-09 at 18:21:50 UTC, Lee Jones wrote:
> Fixes the following W=1 kernel build warning(s):
> 
>  drivers/mtd/spi-nor/controllers/hisi-sfc.c:328: warning: Function parameter 
> or member 'np' not described in 'hisi_spi_nor_register'
>  drivers/mtd/spi-nor/controllers/hisi-sfc.c:328: warning: Function parameter 
> or member 'host' not described in 'hisi_spi_nor_register'
> 
> Cc: Tudor Ambarus 
> Cc: Miquel Raynal 
> Cc: Richard Weinberger 
> Cc: Vignesh Raghavendra 
> Cc: Sumit Semwal 
> Cc: "Christian König" 
> Cc: linux-...@lists.infradead.org
> Cc: linux-me...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linaro-mm-...@lists.linaro.org
> Signed-off-by: Lee Jones 
> Reviewed-by: Vignesh Raghavendra 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
nand/next, thanks.

Miquel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 00/23] Rid W=1 warnings in MTD

2020-11-09 Thread Miquel Raynal
Hi Lee,

Lee Jones  wrote on Fri,  6 Nov 2020 21:36:32
+:

> This set is part of a larger effort attempting to clean-up W=1
> kernel builds, which are currently overwhelmingly riddled with
> niggly little warnings.
> 
> v1 => v2:
>   - Added tags
>   - Satisfied Miquel's review comments
> 

You probably missed my request to update the titles. That's why I
wanted the entire series to be resent. Anyway, as I forgot a few,
please find below the prefixes that should be used:

> Lee Jones (23):
>   mtd: mtdpart: Fix misdocumented function parameter 'mtd'
>   mtd: devices: phram: File headers are not good candidates for
> kernel-doc
>   mtd: nand: onenand: onenand_base: Fix expected kernel-doc formatting

mtd: onenand: Fix...

>   mtd: devices: docg3: Fix kernel-doc 'bad line' and 'excessive doc'
> issues
>   mtd: mtdcore: Fix misspelled function parameter 'section'

mtd: Fix...

>   mtd: nand: onenand: onenand_bbt: Fix expected kernel-doc formatting

mtd: onenand: Fix...

>   mtd: spi-nor: controllers: hisi-sfc: Demote non-conformant kernel-doc

mtd: spi-nor: hisi-sfc: Demote...

>   mtd: ubi: build: Document 'ubi_num' in struct mtd_dev_param
>   mtd: nand: spi: toshiba: Demote non-conformant kernel-doc header

mtd: spinand: toshiba: Demote...

>   mtd: ubi: kapi: Correct documentation for 'ubi_leb_read_sg's 'sgl'
> parameter
>   mtd: ubi: eba: Fix a couple of misdocumentation issues
>   mtd: ubi: wl: Fix a couple of kernel-doc issues
>   mtd: nand: raw: brcmnand: brcmnand: Demote non-conformant kernel-doc
> headers

mtd: rawnand: brcmnand: Demote...

>   mtd: ubi: gluebi: Fix misnamed function parameter documentation
>   mtd: nand: raw: diskonchip: Marking unused variables as
> __always_unused

mtd: rawnand: diskonchip: Marking...

>   mtd: nand: raw: cafe_nand: Remove superfluous param doc and add
> another

mtd: rawnand: cafe: Remove

>   mtd: nand: raw: s3c2410: Add documentation for 2 missing struct
> members

mtd: rawnand: s3c2410: Add...

>   mtd: nand: raw: omap_elm: Finish half populated function header,
> demote empty ones

mtd: rawnand: omap_elm: Finish

>   mtd: nand: raw: omap2: Fix a bunch of kernel-doc misdemeanours

mtd:r rawnand: omap2: Fix

>   mtd: nand: raw: sunxi_nand: Document 'sunxi_nfc's 'caps' member

mtd: rawnand: sunxi: Document

>   mtd: nand: raw: arasan-nand-controller: Document 'anfc_op's 'buf'
> member

mtd: rawnand: arasan: Document

>   mtd: nand: onenand: onenand_base: Fix some kernel-doc misdemeanours

mtd: onenand: Fix

>   mtd: devices: powernv_flash: Add function names to headers and fix
> 'dev'

Otherwise the content of the series looks good to me.

Thanks,
Miquèl
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH][next] drm/rockchip: lvds: ensure ret is assigned before checking for an error

2020-08-03 Thread Miquel Raynal
Hello,

Colin King  wrote on Tue, 14 Jul 2020
20:00:03 +0100:

> From: Colin Ian King 
> 
> Currently there are two places where the return status in ret is being
> checked for an error however the assignment of ret has been omitted
> making the checks redundant.  Fix this by adding in the missing assignments
> of ret.
> 
> Addresses-Coverity: ("Logically dead code")
> Fixes: cca1705c3d89 ("drm/rockchip: lvds: Add PX30 support")
> Signed-off-by: Colin Ian King 
> ---
>  drivers/gpu/drm/rockchip/rockchip_lvds.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c 
> b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> index 63f967902c2d..b45c618b9793 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> @@ -499,11 +499,11 @@ static int px30_lvds_probe(struct platform_device *pdev,
>   if (IS_ERR(lvds->dphy))
>   return PTR_ERR(lvds->dphy);
>  
> - phy_init(lvds->dphy);
> + ret = phy_init(lvds->dphy);
>   if (ret)
>   return ret;
>  
> - phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
> + ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
>   if (ret)
>   return ret;
>  

I thought I (or Heiko) already sent a patch for that but apparently
not...

Reviewed-by: Miquel Raynal 

Thanks,
Miquèl
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 2/4] drm: panel: simple: Correct bus format for Satoz SAT050AT40H12R2

2020-06-30 Thread Miquel Raynal


Laurent Pinchart  wrote on
Tue, 30 Jun 2020 02:33:18 +0300:

> The Satoz SAT050AT40H12R2 panel is an LVDS panel, the
> MEDIA_BUS_FMT_RGB888_1X24 bus format is thus incorrect. Set it to the
> correct value MEDIA_BUS_FMT_RGB888_1X7X4_SPWG.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c 
> b/drivers/gpu/drm/panel/panel-simple.c
> index ac6e8d55a3a2..c659d8262e5c 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -3118,7 +3118,7 @@ static const struct panel_desc satoz_sat050at40h12r2 = {
>   .width = 108,
>   .height = 65,
>   },
> - .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> + .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
>   .connector_type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  

Reviewed-by: Miquel Raynal 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v5 2/2] drm/panel: simple: Add Satoz SAT050AT40H12R2 panel support

2020-06-30 Thread Miquel Raynal
Hi Laurent,

[...]

> >  
> > +static const struct display_timing satoz_sat050at40h12r2_timing = {
> > +   .pixelclock = {3330, 3330, 5000},
> > +   .hactive = {800, 800, 800},
> > +   .hfront_porch = {16, 210, 354},
> > +   .hback_porch = {46, 46, 46},
> > +   .hsync_len = {1, 1, 40},
> > +   .vactive = {480, 480, 480},
> > +   .vfront_porch = {7, 22, 147},
> > +   .vback_porch = {23, 23, 23},
> > +   .vsync_len = {1, 1, 20},
> > +};
> > +
> > +static const struct panel_desc satoz_sat050at40h12r2 = {
> > +   .timings = _sat050at40h12r2_timing,
> > +   .num_timings = 1,
> > +   .bpc = 8,
> > +   .size = {
> > +   .width = 108,
> > +   .height = 65,
> > +   },
> > +   .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> > +   .connector_type = DRM_MODE_CONNECTOR_LVDS,  
> 
> I'm trying to fix inconsistencies in the panel-simple driver, and this
> caught my eyes. MEDIA_BUS_FMT_RGB888_1X24 isn't a correct format for
> LVDS panels. MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> MEDIA_BUS_FMT_RGB888_1X7X4_SPWG or MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA
> should be used instead. As I couldn't find documentation for the panel,
> I can't tell which format is correct. Could you please help ?

Indeed, I got this datasheet under NDA.

We checked with Paul, we think the right format is:
MEDIA_BUS_FMT_RGB888_1X7X4_SPWG.

Thanks,
Miquèl
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 1/2] mtd: rawnand: brcmnand: fix hamming oob layout

2020-05-25 Thread Miquel Raynal
On Tue, 2020-05-12 at 07:57:32 UTC, 
=?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote:
> First 2 bytes are used in large-page nand.
> 
> Fixes: ef5eeea6e911 ("mtd: nand: brcm: switch to mtd_ooblayout_ops")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Álvaro Fernández Rojas 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
nand/next, thanks.

Miquel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 4/5] dt: bindings: brcmnand: add v2.1 and v2.2 support

2020-05-25 Thread Miquel Raynal

Álvaro Fernández Rojas  wrote on Sun, 24 May 2020
21:13:41 +:

> Thanks for merging the patches :).
> 
> BTW, is there something wrong with patch 5?
> I can see patches 1-4 applied in 
> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/log/?h=nand/next,
>  but I can’t see patch 5.
> 

Done :)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 1/5] mtd: rawnand: brcmnand: rename v4 registers

2020-05-25 Thread Miquel Raynal
On Fri, 2020-05-22 at 12:15:20 UTC, 
=?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote:
> These registers are also used on v3.3.
> 
> Signed-off-by: Álvaro Fernández Rojas 
> Reviewed-by: Miquel Raynal 
> Acked-by: Florian Fainelli 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
nand/next, thanks.

Miquel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 3/5] mtd: rawnand: brcmnand: rename page sizes

2020-05-25 Thread Miquel Raynal
On Fri, 2020-05-22 at 12:15:22 UTC, 
=?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote:
> Current pages sizes apply to controllers after v3.4
> 
> Signed-off-by: Álvaro Fernández Rojas 
> Acked-by: Florian Fainelli 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
nand/next, thanks.

Miquel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4] mtd: rawnand: brcmnand: correctly verify erased pages

2020-05-25 Thread Miquel Raynal
On Tue, 2020-05-12 at 08:24:51 UTC, 
=?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote:
> The current code checks that the whole OOB area is erased.
> This is a problem when JFFS2 cleanmarkers are added to the OOB, since it will
> fail due to the usable OOB bytes not being 0xff.
> Correct this by only checking that data and ECC bytes aren't 0xff.
> 
> Fixes: 02b88eea9f9c ("mtd: brcmnand: Add check for erased page bitflips")
> Signed-off-by: Álvaro Fernández Rojas 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
nand/next, thanks.

Miquel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 5/5] mtd: rawnand: brcmnand: support v2.1-v2.2 controllers

2020-05-25 Thread Miquel Raynal
On Fri, 2020-05-22 at 12:15:24 UTC, 
=?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote:
> v2.1: tested on Netgear DGND3700v1 (BCM6368)
> v2.2: tested on Netgear DGND3700v2 (BCM6362)
> 
> Signed-off-by: Álvaro Fernández Rojas 
> Acked-by: Florian Fainelli 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
nand/next, thanks.

Miquel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 2/2] mtd: rawnand: brcmnand: improve hamming oob layout

2020-05-25 Thread Miquel Raynal
On Tue, 2020-05-12 at 07:57:33 UTC, 
=?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote:
> The current code generates 8 oob sections:
> S11-5
> ECC   6-8
> S29-15
> S316-21
> ECC   22-24
> S425-31
> S532-37
> ECC   38-40
> S641-47
> S748-53
> ECC   54-56
> S857-63
> 
> Change it by merging continuous sections:
> S11-5
> ECC   6-8
> S29-21
> ECC   22-24
> S325-37
> ECC   38-40
> S441-53
> ECC   54-56
> S557-63
> 
> Signed-off-by: Álvaro Fernández Rojas 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
nand/next, thanks.

Miquel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 2/5] mtd: rawnand: brcmnand: fix CS0 layout

2020-05-25 Thread Miquel Raynal
On Fri, 2020-05-22 at 12:15:21 UTC, 
=?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote:
> Only v3.3-v5.0 have a different CS0 layout.
> Controllers before v3.3 use the same layout for every CS.
> 
> Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB 
> NAND controller")
> Signed-off-by: Álvaro Fernández Rojas 
> Acked-by: Florian Fainelli 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
nand/next, thanks.

Miquel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 4/5] dt: bindings: brcmnand: add v2.1 and v2.2 support

2020-05-25 Thread Miquel Raynal
On Fri, 2020-05-22 at 12:15:23 UTC, 
=?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote:
> Added brcm,brcmnand-v2.1 and brcm,brcmnand-v2.2 as possible compatible
> strings to support brcmnand controllers v2.1 and v2.2.
> 
> Signed-off-by: Álvaro Fernández Rojas 
> Acked-by: Florian Fainelli 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
nand/next, thanks.

Miquel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 4/5] dt: bindings: brcmnand: add v2.1 and v2.2 support

2020-05-25 Thread Miquel Raynal

Miquel Raynal  wrote on Sun, 24 May 2020
21:25:50 +0200:

> On Fri, 2020-05-22 at 12:15:23 UTC, 
> =?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote:
> > Added brcm,brcmnand-v2.1 and brcm,brcmnand-v2.2 as possible compatible
> > strings to support brcmnand controllers v2.1 and v2.2.
> > 
> > Signed-off-by: Álvaro Fernández Rojas 
> > Acked-by: Florian Fainelli   
> 
> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
> nand/next, thanks.

FYI I updated the commit log:
s/dt: bindings:/dt-bindings: mtd:/
s/Added/Add/

Thanks,
Miquèl
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 5/5] nand: brcmnand: support v2.1-v2.2 controllers

2020-05-23 Thread Miquel Raynal
Hi Álvaro,

Álvaro Fernández Rojas  wrote on Fri, 22 May 2020
09:25:25 +0200:

> v2.1: tested on Netgear DGND3700v1 (BCM6368)
> v2.2: tested on Netgear DGND3700v2 (BCM6362)
> 
> Signed-off-by: Álvaro Fernández Rojas 
> ---
>  v3: fix page size shift for v2.1 controllers.

You changed the subject title too which is not accurate anymore, any
reason to do that? Anything else changed in this series that I am not
aware of?

>  v2: split page sizes rename into a different patch.
>  name all block and page sizes versions.
> 
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 85 +---
>  1 file changed, 76 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c 
> b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index ef60dbbeac2b..2c8a468c2e38 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -264,6 +264,7 @@ struct brcmnand_controller {
>   const unsigned int  *block_sizes;
>   unsigned intmax_page_size;
>   const unsigned int  *page_sizes;
> + unsigned intpage_size_shift;
>   unsigned intmax_oob;
>   u32 features;
>  
> @@ -338,6 +339,36 @@ enum brcmnand_reg {
>   BRCMNAND_FC_BASE,
>  };
>  
> +/* BRCMNAND v2.1-v2.2 */
> +static const u16 brcmnand_regs_v21[] = {
> + [BRCMNAND_CMD_START]=  0x04,
> + [BRCMNAND_CMD_EXT_ADDRESS]  =  0x08,
> + [BRCMNAND_CMD_ADDRESS]  =  0x0c,
> + [BRCMNAND_INTFC_STATUS] =  0x5c,
> + [BRCMNAND_CS_SELECT]=  0x14,
> + [BRCMNAND_CS_XOR]   =  0x18,
> + [BRCMNAND_LL_OP]= 0,
> + [BRCMNAND_CS0_BASE] =  0x40,
> + [BRCMNAND_CS1_BASE] = 0,
> + [BRCMNAND_CORR_THRESHOLD]   = 0,
> + [BRCMNAND_CORR_THRESHOLD_EXT]   = 0,
> + [BRCMNAND_UNCORR_COUNT] = 0,
> + [BRCMNAND_CORR_COUNT]   = 0,
> + [BRCMNAND_CORR_EXT_ADDR]=  0x60,
> + [BRCMNAND_CORR_ADDR]=  0x64,
> + [BRCMNAND_UNCORR_EXT_ADDR]  =  0x68,
> + [BRCMNAND_UNCORR_ADDR]  =  0x6c,
> + [BRCMNAND_SEMAPHORE]=  0x50,
> + [BRCMNAND_ID]   =  0x54,
> + [BRCMNAND_ID_EXT]   = 0,
> + [BRCMNAND_LL_RDATA] = 0,
> + [BRCMNAND_OOB_READ_BASE]=  0x20,
> + [BRCMNAND_OOB_READ_10_BASE] = 0,
> + [BRCMNAND_OOB_WRITE_BASE]   =  0x30,
> + [BRCMNAND_OOB_WRITE_10_BASE]= 0,
> + [BRCMNAND_FC_BASE]  = 0x200,
> +};
> +
>  /* BRCMNAND v3.3-v4.0 */
>  static const u16 brcmnand_regs_v33[] = {
>   [BRCMNAND_CMD_START]=  0x04,
> @@ -536,6 +567,9 @@ enum {
>   CFG_BUS_WIDTH   = BIT(CFG_BUS_WIDTH_SHIFT),
>   CFG_DEVICE_SIZE_SHIFT   = 24,
>  
> + /* Only for v2.1 */
> + CFG_PAGE_SIZE_SHIFT_v2_1= 30,
> +
>   /* Only for pre-v7.1 (with no CFG_EXT register) */
>   CFG_PAGE_SIZE_SHIFT = 20,
>   CFG_BLK_SIZE_SHIFT  = 28,
> @@ -571,12 +605,16 @@ static int brcmnand_revision_init(struct 
> brcmnand_controller *ctrl)
>  {
>   static const unsigned int block_sizes_v6[] = { 8, 16, 128, 256, 512, 
> 1024, 2048, 0 };
>   static const unsigned int block_sizes_v4[] = { 16, 128, 8, 512, 256, 
> 1024, 2048, 0 };
> + static const unsigned int block_sizes_v2_2[] = { 16, 128, 8, 512, 256, 
> 0 };
> + static const unsigned int block_sizes_v2_1[] = { 16, 128, 8, 512, 0 };
>   static const unsigned int page_sizes_v3_4[] = { 512, 2048, 4096, 8192, 
> 0 };
> + static const unsigned int page_sizes_v2_2[] = { 512, 2048, 4096, 0 };
> + static const unsigned int page_sizes_v2_1[] = { 512, 2048, 0 };
>  
>   ctrl->nand_version = nand_readreg(ctrl, 0) & 0x;
>  
> - /* Only support v4.0+? */
> - if (ctrl->nand_version < 0x0400) {
> + /* Only support v2.1+ */
> + if (ctrl->nand_version < 0x0201) {
>   dev_err(ctrl->dev, "version %#x not supported\n",
>   ctrl->nand_version);
>   return -ENODEV;
> @@ -593,6 +631,8 @@ static int brcmnand_revision_init(struct 
> brcmnand_controller *ctrl)
>   ctrl->reg_offsets = brcmnand_regs_v50;
>   else if (ctrl->nand_version >= 0x0303)
>   ctrl->reg_offsets = brcmnand_regs_v33;
> + else if (ctrl->nand_version >= 0x0201)
> + ctrl->reg_offsets = brcmnand_regs_v21;
>  
>   /* Chip-select stride */
>   if (ctrl->nand_version >= 0x0701)
> @@ -618,14 +658,32 @@ static int brcmnand_revision_init(struct 
> brcmnand_controller *ctrl)
>   ctrl->max_page_size = 16 * 1024;
>   ctrl->max_block_size = 2 * 1024 * 1024;
>   } else {
> - ctrl->page_sizes = page_sizes_v3_4;
> + if (ctrl->nand_version >= 0x0304)
> + 

Re: [PATCH v3] mtd: rawnand: brcmnand: correctly verify erased pages

2020-05-13 Thread Miquel Raynal
Hi Álvaro,

Álvaro Fernández Rojas  wrote on Tue, 12 May 2020
09:24:32 +0200:

> Hi Miquèl
> 
> > El 12 may 2020, a las 9:16, Miquel Raynal  
> > escribió:
> > 
> > Hi Álvaro,
> > 
> > Álvaro Fernández Rojas  wrote on Tue, 12 May 2020
> > 08:51:11 +0200:
> >   
> >> The current code checks that the whole OOB area is erased.
> >> This is a problem when JFFS2 cleanmarkers are added to the OOB, since it 
> >> will
> >> fail due to the usable OOB bytes not being 0xff.
> >> Correct this by only checking that data and ECC bytes aren't 0xff.
> >> 
> >> Fixes: 02b88eea9f9c ("mtd: brcmnand: Add check for erased page bitflips")
> >> Signed-off-by: Álvaro Fernández Rojas 
> >> ---
> >> v3: Fix commit log and merge nand_check_erased_ecc_chunk calls.
> >> v2: Add Fixes tag
> >> 
> >> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 19 ++-
> >> 1 file changed, 14 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c 
> >> b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >> index e4e3ceeac38f..80fe01f03516 100644
> >> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >> @@ -2018,8 +2018,9 @@ static int brcmnand_read_by_pio(struct mtd_info 
> >> *mtd, struct nand_chip *chip,
> >> static int brcmstb_nand_verify_erased_page(struct mtd_info *mtd,
> >>  struct nand_chip *chip, void *buf, u64 addr)
> >> {
> >> +  struct mtd_oob_region oobecc;
> >>int i, sas;
> >> -  void *oob = chip->oob_poi;
> >> +  void *oob;
> >>int bitflips = 0;
> >>int page = addr >> chip->page_shift;
> >>int ret;
> >> @@ -2035,11 +2036,19 @@ static int brcmstb_nand_verify_erased_page(struct 
> >> mtd_info *mtd,
> >>if (ret)
> >>return ret;
> >> 
> >> -  for (i = 0; i < chip->ecc.steps; i++, oob += sas) {
> >> +  for (i = 0; i < chip->ecc.steps; i++) {
> >>ecc_chunk = buf + chip->ecc.size * i;
> >> -  ret = nand_check_erased_ecc_chunk(ecc_chunk,
> >> -chip->ecc.size,
> >> -oob, sas, NULL, 0,
> >> +
> >> +  if (mtd->ooblayout->ecc(mtd, i, )) {  
> > 
> > Please use the mtdcore.c's helpers
> > (mtd_ooblayout_set/get_data/free/ecc/bytes).
> > 
> > Also, what are you trying to discriminate with the return code of the
> > function? Shouldn't this function "always" work?  
> 
> Just making sure it doesn’t return an ERANGE in case chip->ecc.size doesn’t 
> match the sections from mtd->ooblayout->ecc, which shouldn’t happen, so I 
> think we can remove that...

The style we prefer for error checking is:

ret = function();
if (ret)
do someting;

instead of:

if (function())

Anyway, I really don't know if it can happen or not. I suppose it does.
What I don't understand is your "oob = chip->oob_poi + oobecc.offset".
If you expect an error, then you should not update this pointer, right?

Don't you need to use 2 * i instead of i here? Following your other
contribution, sections are distributed like "data/ecc/data/ecc/etc".

> 
> >   
> >> +  oob = NULL;
> >> +  oobecc.length = 0;
> >> +  } else {
> >> +  oob = chip->oob_poi + oobecc.offset;
> >> +  }
> >> +
> >> +  ret = nand_check_erased_ecc_chunk(ecc_chunk, chip->ecc.size,
> >> +oob, oobecc.length,
> >> +NULL, 0,
> >>  chip->ecc.strength);  
> > 
> > As I told you, this helper takes "maid data" then "spare area" then
> > "ecc bytes". The names are pretty important here as you want to avoid
> > checking the spare OOB bytes on purpose, so maybe you could have more
> > meaningful names and call "ecc" instead of "oob" the ecc region?  
> 
> Actually I thought you meant the commit log, not the code itself...

No problem ;) I meant both actually, And I think you should name the
oob pointer ecc_bytes.

> 
> >   
> >>if (ret < 0)
> >>return ret;  
> > 
> > 
> > Thanks,
> > Miquèl  
> 
> Regards,
> Álvaro.
> 




Thanks,
Miquèl
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 2/2] mtd: rawnand: brcmnand: improve hamming oob layout

2020-05-13 Thread Miquel Raynal
Hi Álvaro,

Álvaro Fernández Rojas  wrote on Tue, 12 May 2020
09:12:10 +0200:

> Hi Miquel,
> 
> I also had a hard time understanding your email.
> It was quite misleading.
> 
> > El 12 may 2020, a las 9:08, Miquel Raynal  
> > escribió:
> > 
> > Hi Álvaro,
> > 
> > Álvaro Fernández Rojas  wrote on Tue, 12 May 2020
> > 08:00:23 +0200:
> >   
> >> The current code generates 8 oob sections:
> >> S1 1-5
> >> ECC6-8
> >> S2 9-15
> >> S3 16-21
> >> ECC22-24
> >> S4 25-31
> >> S5 32-37
> >> ECC38-40
> >> S6 41-47
> >> S7 48-53
> >> ECC54-56
> >> S8 57-63
> >> 
> >> Change it by merging continuous sections:
> >> S1 1-5
> >> ECC6-8
> >> S2 9-21
> >> ECC22-24
> >> S3 25-37
> >> ECC38-40
> >> S4 41-53
> >> ECC54-56
> >> S5 57-63
> >> 
> >> Fixes: ef5eeea6e911 ("mtd: nand: brcm: switch to mtd_ooblayout_ops")  
> > 
> > Sorry for leading you the wrong way, actually this patch does not
> > deserve a Fixes tag.  
> 
> Do I need to resend this again?
> Looks like no matter what I do it’s always wrong...

Please don't give up! It is normal to work back and forth with the
community. I need the patch to be clear and bug-free so I ask you to
make changes and ask questions, that's how it works. But all your
patches are enhancing this driver so please keep posting!

> 
> >   
> >> Signed-off-by: Álvaro Fernández Rojas 
> >> ---
> >> v3: invert patch order
> >> v2: keep original comment and fix correctly skip byte 6 for small-page nand
> >> 
> >> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 37 
> >> 1 file changed, 18 insertions(+), 19 deletions(-)
> >> 
> >> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c 
> >> b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >> index 1c1070111ebc..0a1d76fde37b 100644
> >> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >> @@ -1100,33 +1100,32 @@ static int brcmnand_hamming_ooblayout_free(struct 
> >> mtd_info *mtd, int section,
> >>struct brcmnand_cfg *cfg = >hwcfg;
> >>int sas = cfg->spare_area_size << cfg->sector_size_1k;
> >>int sectors = cfg->page_size / (512 << cfg->sector_size_1k);
> >> +  u32 next;
> >> 
> >> -  if (section >= sectors * 2)
> >> +  if (section > sectors)
> >>return -ERANGE;
> >> 
> >> -  oobregion->offset = (section / 2) * sas;
> >> +  next = (section * sas);
> >> +  if (section < sectors)
> >> +  next += 6;
> >> 
> >> -  if (section & 1) {
> >> -  oobregion->offset += 9;
> >> -  oobregion->length = 7;
> >> +  if (section) {
> >> +  oobregion->offset = ((section - 1) * sas) + 9;
> >>} else {
> >> -  oobregion->length = 6;
> >> -
> >> -  /* First sector of each page may have BBI */
> >> -  if (!section) {
> >> -  /*
> >> -   * Small-page NAND use byte 6 for BBI while large-page
> >> -   * NAND use bytes 0 and 1.
> >> -   */
> >> -  if (cfg->page_size > 512) {
> >> -  oobregion->offset += 2;
> >> -  oobregion->length -= 2;
> >> -  } else {
> >> -  oobregion->length--;
> >> -  }
> >> +  /*
> >> +   * Small-page NAND use byte 6 for BBI while large-page
> >> +   * NAND use bytes 0 and 1.
> >> +   */
> >> +  if (cfg->page_size > 512) {
> >> +  oobregion->offset = 2;
> >> +  } else {
> >> +  oobregion->offset = 0;
> >> +  next--;  
> > 
> > This next-- seems very strange, can you explain?  
> 
> In this case next will be 6 (which is the first ECC byte).
> However, for small page NANDs byte 5 is reserved for BBT, so we want next to 
> be 5 only in this case.

That's clear, please add a comment there then.

> 
> >   
> >>}
> >>}
> >> 
> >> +  oobregion->length = next - oobregion->offset;
> >> +
> >>return 0;
> >> }
> >>   
> > 
> > 
> > Thanks,
> > Miquèl  
> 
> Regards,
> Álvaro.



Thanks,
Miquèl
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3] mtd: rawnand: brcmnand: correctly verify erased pages

2020-05-13 Thread Miquel Raynal
Hi Álvaro,

Álvaro Fernández Rojas  wrote on Tue, 12 May 2020
08:51:11 +0200:

> The current code checks that the whole OOB area is erased.
> This is a problem when JFFS2 cleanmarkers are added to the OOB, since it will
> fail due to the usable OOB bytes not being 0xff.
> Correct this by only checking that data and ECC bytes aren't 0xff.
> 
> Fixes: 02b88eea9f9c ("mtd: brcmnand: Add check for erased page bitflips")
> Signed-off-by: Álvaro Fernández Rojas 
> ---
>  v3: Fix commit log and merge nand_check_erased_ecc_chunk calls.
>  v2: Add Fixes tag
> 
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 19 ++-
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c 
> b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index e4e3ceeac38f..80fe01f03516 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -2018,8 +2018,9 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, 
> struct nand_chip *chip,
>  static int brcmstb_nand_verify_erased_page(struct mtd_info *mtd,
> struct nand_chip *chip, void *buf, u64 addr)
>  {
> + struct mtd_oob_region oobecc;
>   int i, sas;
> - void *oob = chip->oob_poi;
> + void *oob;
>   int bitflips = 0;
>   int page = addr >> chip->page_shift;
>   int ret;
> @@ -2035,11 +2036,19 @@ static int brcmstb_nand_verify_erased_page(struct 
> mtd_info *mtd,
>   if (ret)
>   return ret;
>  
> - for (i = 0; i < chip->ecc.steps; i++, oob += sas) {
> + for (i = 0; i < chip->ecc.steps; i++) {
>   ecc_chunk = buf + chip->ecc.size * i;
> - ret = nand_check_erased_ecc_chunk(ecc_chunk,
> -   chip->ecc.size,
> -   oob, sas, NULL, 0,
> +
> + if (mtd->ooblayout->ecc(mtd, i, )) {

Please use the mtdcore.c's helpers
(mtd_ooblayout_set/get_data/free/ecc/bytes).

Also, what are you trying to discriminate with the return code of the
function? Shouldn't this function "always" work?

> + oob = NULL;
> + oobecc.length = 0;
> + } else {
> + oob = chip->oob_poi + oobecc.offset;
> + }
> +
> + ret = nand_check_erased_ecc_chunk(ecc_chunk, chip->ecc.size,
> +   oob, oobecc.length,
> +   NULL, 0,
> chip->ecc.strength);

As I told you, this helper takes "maid data" then "spare area" then
"ecc bytes". The names are pretty important here as you want to avoid
checking the spare OOB bytes on purpose, so maybe you could have more
meaningful names and call "ecc" instead of "oob" the ecc region?
 
>   if (ret < 0)
>   return ret;


Thanks,
Miquèl
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 2/2] mtd: rawnand: brcmnand: improve hamming oob layout

2020-05-13 Thread Miquel Raynal
Hi Álvaro,

Álvaro Fernández Rojas  wrote on Tue, 12 May 2020
09:26:23 +0200:

> Hi Miquèl,
> 
> > El 12 may 2020, a las 9:19, Miquel Raynal  
> > escribió:
> > 
> > Hi Álvaro,
> > 
> > Álvaro Fernández Rojas  wrote on Tue, 12 May 2020
> > 09:12:10 +0200:
> >   
> >> Hi Miquel,
> >> 
> >> I also had a hard time understanding your email.
> >> It was quite misleading.
> >>   
> >>> El 12 may 2020, a las 9:08, Miquel Raynal  
> >>> escribió:
> >>> 
> >>> Hi Álvaro,
> >>> 
> >>> Álvaro Fernández Rojas  wrote on Tue, 12 May 2020
> >>> 08:00:23 +0200:
> >>>   
> >>>> The current code generates 8 oob sections:
> >>>> S1   1-5
> >>>> ECC  6-8
> >>>> S2   9-15
> >>>> S3   16-21
> >>>> ECC  22-24
> >>>> S4   25-31
> >>>> S5   32-37
> >>>> ECC  38-40
> >>>> S6   41-47
> >>>> S7   48-53
> >>>> ECC  54-56
> >>>> S8   57-63
> >>>> 
> >>>> Change it by merging continuous sections:
> >>>> S1   1-5
> >>>> ECC  6-8
> >>>> S2   9-21
> >>>> ECC  22-24
> >>>> S3   25-37
> >>>> ECC  38-40
> >>>> S4   41-53
> >>>> ECC  54-56
> >>>> S5   57-63
> >>>> 
> >>>> Fixes: ef5eeea6e911 ("mtd: nand: brcm: switch to mtd_ooblayout_ops")
> >>> 
> >>> Sorry for leading you the wrong way, actually this patch does not
> >>> deserve a Fixes tag.
> >> 
> >> Do I need to resend this again?
> >> Looks like no matter what I do it’s always wrong...  
> > 
> > Please don't give up! It is normal to work back and forth with the
> > community. I need the patch to be clear and bug-free so I ask you to
> > make changes and ask questions, that's how it works. But all your
> > patches are enhancing this driver so please keep posting!
> >   
> >>   
> >>>   
> >>>> Signed-off-by: Álvaro Fernández Rojas 
> >>>> ---
> >>>> v3: invert patch order
> >>>> v2: keep original comment and fix correctly skip byte 6 for small-page 
> >>>> nand
> >>>> 
> >>>> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 37 
> >>>> 1 file changed, 18 insertions(+), 19 deletions(-)
> >>>> 
> >>>> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c 
> >>>> b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >>>> index 1c1070111ebc..0a1d76fde37b 100644
> >>>> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >>>> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >>>> @@ -1100,33 +1100,32 @@ static int 
> >>>> brcmnand_hamming_ooblayout_free(struct mtd_info *mtd, int section,
> >>>>  struct brcmnand_cfg *cfg = >hwcfg;
> >>>>  int sas = cfg->spare_area_size << cfg->sector_size_1k;
> >>>>  int sectors = cfg->page_size / (512 << cfg->sector_size_1k);
> >>>> +u32 next;
> >>>> 
> >>>> -if (section >= sectors * 2)
> >>>> +if (section > sectors)
> >>>>  return -ERANGE;
> >>>> 
> >>>> -oobregion->offset = (section / 2) * sas;
> >>>> +next = (section * sas);
> >>>> +if (section < sectors)
> >>>> +next += 6;
> >>>> 
> >>>> -if (section & 1) {
> >>>> -oobregion->offset += 9;
> >>>> -oobregion->length = 7;
> >>>> +if (section) {
> >>>> +oobregion->offset = ((section - 1) * sas) + 9;
> >>>>  } else {
> >>>> -oobregion->length = 6;
> >>>> -
> >>>> -/* First sector of each page may have BBI */
> >>>> -if (!section) {
> >>>> -/*
> >>>> - * Small-page NAND use byte 6 for BBI while 
> >>>> large-page
> >>>> - * NAND use bytes 0 and 1.
> >>>

Re: [PATCH v3 2/2] mtd: rawnand: brcmnand: improve hamming oob layout

2020-05-13 Thread Miquel Raynal
Hi Álvaro,

Álvaro Fernández Rojas  wrote on Tue, 12 May 2020
08:00:23 +0200:

> The current code generates 8 oob sections:
> S11-5
> ECC   6-8
> S29-15
> S316-21
> ECC   22-24
> S425-31
> S532-37
> ECC   38-40
> S641-47
> S748-53
> ECC   54-56
> S857-63
> 
> Change it by merging continuous sections:
> S11-5
> ECC   6-8
> S29-21
> ECC   22-24
> S325-37
> ECC   38-40
> S441-53
> ECC   54-56
> S557-63
> 
> Fixes: ef5eeea6e911 ("mtd: nand: brcm: switch to mtd_ooblayout_ops")

Sorry for leading you the wrong way, actually this patch does not
deserve a Fixes tag.

> Signed-off-by: Álvaro Fernández Rojas 
> ---
>  v3: invert patch order
>  v2: keep original comment and fix correctly skip byte 6 for small-page nand
> 
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 37 
>  1 file changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c 
> b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index 1c1070111ebc..0a1d76fde37b 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -1100,33 +1100,32 @@ static int brcmnand_hamming_ooblayout_free(struct 
> mtd_info *mtd, int section,
>   struct brcmnand_cfg *cfg = >hwcfg;
>   int sas = cfg->spare_area_size << cfg->sector_size_1k;
>   int sectors = cfg->page_size / (512 << cfg->sector_size_1k);
> + u32 next;
>  
> - if (section >= sectors * 2)
> + if (section > sectors)
>   return -ERANGE;
>  
> - oobregion->offset = (section / 2) * sas;
> + next = (section * sas);
> + if (section < sectors)
> + next += 6;
>  
> - if (section & 1) {
> - oobregion->offset += 9;
> - oobregion->length = 7;
> + if (section) {
> + oobregion->offset = ((section - 1) * sas) + 9;
>   } else {
> - oobregion->length = 6;
> -
> - /* First sector of each page may have BBI */
> - if (!section) {
> - /*
> -  * Small-page NAND use byte 6 for BBI while large-page
> -  * NAND use bytes 0 and 1.
> -  */
> - if (cfg->page_size > 512) {
> - oobregion->offset += 2;
> - oobregion->length -= 2;
> - } else {
> - oobregion->length--;
> - }
> + /*
> +  * Small-page NAND use byte 6 for BBI while large-page
> +  * NAND use bytes 0 and 1.
> +  */
> + if (cfg->page_size > 512) {
> + oobregion->offset = 2;
> + } else {
> + oobregion->offset = 0;
> + next--;

This next-- seems very strange, can you explain?

>   }
>   }
>  
> + oobregion->length = next - oobregion->offset;
> +
>   return 0;
>  }
>  


Thanks,
Miquèl
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH V4 3/3] mtd: rawnand: brcmnand: Add support for flash-edu for dma transfers

2020-03-12 Thread Miquel Raynal
On Wed, 2020-01-22 at 21:33:13 UTC, Kamal Dasu wrote:
> Legacy mips soc platforms that have controller v5.0 and 6.0 use
> flash-edu block for dma transfers. This change adds support for
> nand dma transfers using the EDU block.
> 
> Signed-off-by: Kamal Dasu 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
nand/next, thanks.

Miquel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH V4 1/3] dt: bindings: brcmnand: Add support for flash-edu

2020-03-12 Thread Miquel Raynal
On Wed, 2020-01-22 at 21:33:11 UTC, Kamal Dasu wrote:
> Adding support for EBI DMA unit (EDU).
> 
> Signed-off-by: Kamal Dasu 
> Reviewed-by: Rob Herring 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
nand/next, thanks.

Miquel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH -next 013/491] INGENIC JZ47xx SoCs: Use fallthrough;

2020-03-12 Thread Miquel Raynal

Ulf Hansson  wrote on Wed, 11 Mar 2020 15:20:59
+0100:

> On Wed, 11 Mar 2020 at 08:40, Miquel Raynal  wrote:
> >
> > Hi Joe,
> >
> > Joe Perches  wrote on Tue, 10 Mar 2020 21:51:27 -0700:
> >  
> > > Convert the various uses of fallthrough comments to fallthrough;
> > >
> > > Done via script
> > > Link: 
> > > https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/
> > >
> > > Signed-off-by: Joe Perches 
> > > ---
> > >  drivers/gpu/drm/ingenic/ingenic-drm.c   | 2 +-
> > >  drivers/mmc/host/jz4740_mmc.c   | 6 ++
> > >  drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c | 2 +-
> > >  drivers/mtd/nand/raw/ingenic/jz4725b_bch.c  | 4 ++--
> > >  drivers/mtd/nand/raw/ingenic/jz4780_bch.c   | 4 ++--
> > >  sound/soc/codecs/jz4770.c   | 2 +-
> > >  6 files changed, 9 insertions(+), 11 deletions(-)  
> >
> > I like very much the new way to advertise for fallthrough statements,
> > but I am not willing to take any patch converting a single driver
> > anymore. I had too many from Gustavo when these comments had to be
> > inserted. I would really prefer a MTD-wide or a NAND-wide or at least a
> > raw-NAND-wide single patch (anything inside drivers/mtd/nand/raw/).
> >
> > Hope you'll understand!  
> 
> I fully agree (for mmc). One patch please.
> 
> Another option is to make a big fat tree wide patch and ask Linus if
> he want to pick up immediately after an rc1. That should cause less
> disturbance for everyone, no?

Absolutely.

Miquèl
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH V4 2/3] arch: mips: brcm: Add 7425 flash-edu support

2020-03-12 Thread Miquel Raynal
On Wed, 2020-01-22 at 21:33:12 UTC, Kamal Dasu wrote:
> Nand controller v5.0 and v6.0 have nand edu blocks that enable
> dma nand flash transfers. This allows for faster read and write
> access.
> 
> Signed-off-by: Kamal Dasu 
> Acked-by: Paul Burton 
> Reviewed-by: Florian Fainelli 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
nand/next, thanks.

Miquel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH -next 013/491] INGENIC JZ47xx SoCs: Use fallthrough;

2020-03-11 Thread Miquel Raynal
Hi Joe,

Joe Perches  wrote on Tue, 10 Mar 2020 21:51:27 -0700:

> Convert the various uses of fallthrough comments to fallthrough;
> 
> Done via script
> Link: 
> https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/
>
> Signed-off-by: Joe Perches 
> ---
>  drivers/gpu/drm/ingenic/ingenic-drm.c   | 2 +-
>  drivers/mmc/host/jz4740_mmc.c   | 6 ++
>  drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c | 2 +-
>  drivers/mtd/nand/raw/ingenic/jz4725b_bch.c  | 4 ++--
>  drivers/mtd/nand/raw/ingenic/jz4780_bch.c   | 4 ++--
>  sound/soc/codecs/jz4770.c   | 2 +-
>  6 files changed, 9 insertions(+), 11 deletions(-)

I like very much the new way to advertise for fallthrough statements,
but I am not willing to take any patch converting a single driver
anymore. I had too many from Gustavo when these comments had to be
inserted. I would really prefer a MTD-wide or a NAND-wide or at least a
raw-NAND-wide single patch (anything inside drivers/mtd/nand/raw/).

Hope you'll understand!

Thanks,
Miquèl
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/rockchip: rgb: don't count non-existent devices when determining subdrivers

2020-03-10 Thread Miquel Raynal
Hi Heiko,

Heiko Stuebner  wrote on Tue, 21 Jan 2020 23:48:28
+0100:

> From: Heiko Stuebner 
> 
> rockchip_drm_endpoint_is_subdriver() may also return error codes.
> For example if the target-node is in the disabled state, so no
> platform-device is getting created for it.
> 
> In that case current code would count that as external rgb device,
> which in turn would make probing the rockchip-drm device fail.
> 
> So only count the target as rgb device if the function actually
> returns 0.
> 
> Signed-off-by: Heiko Stuebner 
> ---
>  drivers/gpu/drm/rockchip/rockchip_rgb.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c 
> b/drivers/gpu/drm/rockchip/rockchip_rgb.c
> index ae730275a34f..79a7e60633e0 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_rgb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
> @@ -98,7 +98,8 @@ struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
>   if (of_property_read_u32(endpoint, "reg", _id))
>   endpoint_id = 0;
>  
> - if (rockchip_drm_endpoint_is_subdriver(endpoint) > 0)
> + /* if subdriver (> 0) or error case (< 0), ignore entry */
> + if (rockchip_drm_endpoint_is_subdriver(endpoint) != 0)
>   continue;
>  
>   child_count++;

Reviewed-by: Miquel Raynal 

Thanks,
Miquèl
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/2] dt-bindings: gpu: mali-bifrost: Add Rockchip PX30 compatible

2020-03-03 Thread Miquel Raynal

Heiko Stübner  wrote on Mon, 02 Mar 2020 17:29:02
+0100:

> Am Montag, 2. März 2020, 16:58:07 CET schrieb Miquel Raynal:
> > Rockchip PX30 SoCs feature a Bifrost Mali GPU.
> > 
> > Signed-off-by: Miquel Raynal 
> > ---
> >  Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 1 +  
> 
> already in mainline ;-)
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ad848dd53385b61f3c2b94d3466bf799f31035a7
> 
> 

You should probably setup an automatic answer ;)

I admit that I wrote these patches some time ago when it was not in
mainline and did not check before sending it :$

Cheers,
Miquèl
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 1/2] dt-bindings: gpu: mali-bifrost: Add Rockchip PX30 compatible

2020-03-03 Thread Miquel Raynal
Rockchip PX30 SoCs feature a Bifrost Mali GPU.

Signed-off-by: Miquel Raynal 
---
 Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml 
b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
index 5f1fd6d7ee0f..283ee0c274d1 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
@@ -17,6 +17,7 @@ properties:
 items:
   - enum:
   - amlogic,meson-g12a-mali
+  - rockchip,px30-mali
   - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully 
discoverable
 
   reg:
-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


  1   2   >