Re: [PATCH 2/2] mtd: Remove support for Carillo Ranch driver
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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()
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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()
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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()
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
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
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()
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
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
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
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
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
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
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
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
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
Á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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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;
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
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;
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
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
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
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