Re: [PATCH] drm/ingenic: Fix pixclock rate for 24-bit serial panels
> Am 12.04.2021 um 16:34 schrieb Paul Cercueil :
>
> Hi,
>
> Can I have an ACK for this patch?
>
> Then I can apply it to drm-misc-next-fixes.
>
> Cheers,
> -Paul
>
>
> Le mar. 23 mars 2021 à 14:40, Paul Cercueil a écrit :
>> When using a 24-bit panel on a 8-bit serial bus, the pixel clock
>> requested by the panel has to be multiplied by 3, since the subpixels
>> are shifted sequentially.
>> The code (in ingenic_drm_encoder_atomic_check) already computed
>> crtc_state->adjusted_mode->crtc_clock accordingly, but clk_set_rate()
>> used crtc_state->adjusted_mode->clock instead.
>> Fixes: 28ab7d35b6e0 ("drm/ingenic: Properly compute timings when using a
>> 3x8-bit panel")
>> Cc: sta...@vger.kernel.org # v5.10
Tested-by: H. Nikolaus Schaller # CI20/jz4780 (HDMI)
and Alpha400/jz4730 (LCD)
>> Signed-off-by: Paul Cercueil
>> ---
>> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> index d60e1eefc9d1..cba68bf52ec5 100644
>> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> @@ -342,7 +342,7 @@ static void ingenic_drm_crtc_atomic_flush(struct
>> drm_crtc *crtc,
>> if (priv->update_clk_rate) {
>> mutex_lock(>clk_mutex);
>> clk_set_rate(priv->pix_clk,
>> - crtc_state->adjusted_mode.clock * 1000);
>> + crtc_state->adjusted_mode.crtc_clock * 1000);
>> priv->update_clk_rate = false;
>> mutex_unlock(>clk_mutex);
>> }
>> --
>> 2.30.2
>
>
Re: [BUG]: usb: dwc3: gadget: Prevent EP queuing while stopping transfers
> Am 04.04.2021 um 12:01 schrieb Greg KH :
>
> On Sun, Apr 04, 2021 at 11:29:00AM +0200, H. Nikolaus Schaller wrote:
>> it seems as if the patch
>>
>> 9de47c37 ("usb: dwc3: gadget: Prevent EP queuing while stopping
>> transfers") in v5.11.y
>> f09ddcfcb8c5 ("usb: dwc3: gadget: Prevent EP queuing while stopping
>> transfers") in v5.12-rc5
>>
>> reproducible breaks dwc3 RNDIS gadget, at least on the Pyra Handheld (OMAP5).
>>
>> The symptom of having this patch in tree (v5.11.10 or v5.12) is that
>> rndis/gadget initially works after boot.
>
> Should be fixed now by 5aef629704ad ("usb: dwc3: gadget: Clear DEP flags
> after stop transfers in ep disable"). Can you test and verify this?
Yes it works. I was no longer able to reproduce the console log.
Thanks for the quick response!
BR,
Nikolaus Schaller
[BUG]: usb: dwc3: gadget: Prevent EP queuing while stopping transfers
it seems as if the patch
9de47c37 ("usb: dwc3: gadget: Prevent EP queuing while stopping
transfers") in v5.11.y
f09ddcfcb8c5 ("usb: dwc3: gadget: Prevent EP queuing while stopping
transfers") in v5.12-rc5
reproducible breaks dwc3 RNDIS gadget, at least on the Pyra Handheld (OMAP5).
The symptom of having this patch in tree (v5.11.10 or v5.12) is that
rndis/gadget initially works after boot.
But after unplugging the cable, a replug gives warnings like the following
and RNDIS isn't up and running any more:
[ 72.009811] [ cut here ]
[ 72.014768] WARNING: CPU: 0 PID: 2499 at drivers/usb/dwc3/gadget.c:361
dwc3_send_gadget_ep_cmd+0x1f8/0x330 [dwc3]
[ 72.025846] dwc3 4a03.usb: No resource for ep2in
[ 72.031125] Modules linked in: bnep usb_f_ecm g_ether usb_f_rndis u_ether
libcomposite configfs ipv6 snd_soc_omap_hdmi wl18xx wlcore
panel_boe_btl507212_w677l mac80211 snd_soc_spdif_tx snd_soc_dmic dwc3 roles
cfg80211 libarc4 snd_soc_omap_abe_twl6040 snd_soc_simple_card omapdrm
pvrsrvkm_omap5_sgx544_116 snd_soc_omap_mcpdm snd_soc_simple_card_utils etnaviv
wwan_on_off snd_soc_twl6040 leds_gpio snd_soc_gtm601 drm_kms_helper pwm_bl
pwm_omap_dmtimer ti_tpd12s015 display_connector generic_adc_battery
snd_soc_w2cbw003_bt gpu_sched syscopyarea sysfillrect sysimgblt fb_sys_fops cec
drm drm_panel_orientation_quirks omap_sham omap_aes_driver crypto_engine
omap_crypto ehci_omap dwc3_omap wlcore_sdio snd_soc_ts3a227e ina2xx_adc
leds_is31fl319x tsc2007 bq2429x_charger bq27xxx_battery_i2c bq27xxx_battery
ina2xx as5013 tca8418_keypad twl6040_vibra bmp280_spi clk_twl6040 gpio_twl6040
hci_uart btbcm palmas_pwrbutton palmas_gpadc usb3503 bluetooth bmc150_accel_i2c
bmc150_magn_i2c bmp280_i2c bmg160_
i2c
[ 72.032887] bmc150_accel_core bmc150_magn bmg160_core bmp280 ecdh_generic
bno055 industrialio_triggered_buffer ecc kfifo_buf industrialio
snd_soc_omap_aess snd_soc_omap_mcbsp snd_soc_ti_sdma
[ 72.143285] CPU: 0 PID: 2499 Comm: irq/200-dwc3 Not tainted
5.12.0-rc5-letux-lpae+ #5479
[ 72.151914] Hardware name: Generic OMAP5 (Flattened Device Tree)
[ 72.158300] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[ 72.166595] [] (show_stack) from []
(dump_stack+0x8c/0xac)
[ 72.174325] [] (dump_stack) from [] (__warn+0xcc/0xf4)
[ 72.181668] [] (__warn) from []
(warn_slowpath_fmt+0x70/0x9c)
[ 72.189664] [] (warn_slowpath_fmt) from []
(dwc3_send_gadget_ep_cmd+0x1f8/0x330 [dwc3])
[ 72.200223] [] (dwc3_send_gadget_ep_cmd [dwc3]) from []
(__dwc3_gadget_ep_enable+0x344/0x420 [dwc3])
[ 72.212003] [] (__dwc3_gadget_ep_enable [dwc3]) from []
(dwc3_gadget_ep_enable+0xb8/0xe8 [dwc3])
[ 72.223420] [] (dwc3_gadget_ep_enable [dwc3]) from []
(usb_ep_enable+0x3c/0xd4)
[ 72.233182] [] (usb_ep_enable) from []
(ecm_set_alt+0x48/0x13c [usb_f_ecm])
[ 72.242490] [] (ecm_set_alt [usb_f_ecm]) from []
(composite_setup+0xa88/0x152c [libcomposite])
[ 72.253629] [] (composite_setup [libcomposite]) from []
(dwc3_ep0_delegate_req+0x2c/0x40 [dwc3])
[ 72.264981] [] (dwc3_ep0_delegate_req [dwc3]) from []
(dwc3_ep0_interrupt+0x31c/0x824 [dwc3])
[ 72.276122] [] (dwc3_ep0_interrupt [dwc3]) from []
(dwc3_process_event_buf+0x11c/0xa50 [dwc3])
[ 72.287357] [] (dwc3_process_event_buf [dwc3]) from []
(dwc3_thread_interrupt+0x24/0x3c [dwc3])
[ 72.298679] [] (dwc3_thread_interrupt [dwc3]) from []
(irq_thread_fn+0x1c/0x5c)
[ 72.308440] [] (irq_thread_fn) from []
(irq_thread+0x158/0x1e8)
[ 72.316617] [] (irq_thread) from [] (kthread+0x138/0x148)
[ 72.324254] [] (kthread) from []
(ret_from_fork+0x14/0x34)
[ 72.331968] Exception stack(0xc2e99fb0 to 0xc2e99ff8)
[ 72.337339] 9fa0:
[ 72.346022] 9fc0:
[ 72.354716] 9fe0: 0013
[ 72.361753] ---[ end trace d949466d43afc2cb ]---
[ 72.366675] [ cut here ]
[ 72.371579] WARNING: CPU: 0 PID: 2499 at drivers/usb/gadget/udc/core.c:278
usb_ep_queue+0xe8/0x108
[ 72.381131] Modules linked in: bnep usb_f_ecm g_ether usb_f_rndis u_ether
libcomposite configfs ipv6 snd_soc_omap_hdmi wl18xx wlcore
panel_boe_btl507212_w677l mac80211 snd_soc_spdif_tx snd_soc_dmic dwc3 roles
cfg80211 libarc4 snd_soc_omap_abe_twl6040 snd_soc_simple_card omapdrm
pvrsrvkm_omap5_sgx544_116 snd_soc_omap_mcpdm snd_soc_simple_card_utils etnaviv
wwan_on_off snd_soc_twl6040 leds_gpio snd_soc_gtm601 drm_kms_helper pwm_bl
pwm_omap_dmtimer ti_tpd12s015 display_connector generic_adc_battery
snd_soc_w2cbw003_bt gpu_sched syscopyarea sysfillrect sysimgblt fb_sys_fops cec
drm drm_panel_orientation_quirks omap_sham omap_aes_driver crypto_engine
omap_crypto ehci_omap dwc3_omap wlcore_sdio snd_soc_ts3a227e ina2xx_adc
leds_is31fl319x tsc2007 bq2429x_charger bq27xxx_battery_i2c bq27xxx_battery
ina2xx as5013
Re: [PATCH v3 01/17] cmdline: Add generic function to build command line.
> Am 30.03.2021 um 19:27 schrieb Daniel Walker : > > On Fri, Mar 26, 2021 at 01:44:48PM +, Christophe Leroy wrote: >> This code provides architectures with a way to build command line >> based on what is built in the kernel and what is handed over by the >> bootloader, based on selected compile-time options. >> >> Signed-off-by: Christophe Leroy >> --- >> v3: >> - Addressed comments from Will >> - Added capability to have src == dst >> --- >> include/linux/cmdline.h | 57 + >> 1 file changed, 57 insertions(+) >> create mode 100644 include/linux/cmdline.h >> >> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h >> new file mode 100644 >> index ..dea87edd41be >> --- /dev/null >> +++ b/include/linux/cmdline.h >> @@ -0,0 +1,57 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +#ifndef _LINUX_CMDLINE_H >> +#define _LINUX_CMDLINE_H >> + >> +#include >> + >> +/* Allow architectures to override strlcat, powerpc can't use strings so >> early */ >> +#ifndef cmdline_strlcat >> +#define cmdline_strlcat strlcat >> +#endif >> + >> +/* >> + * This function will append or prepend a builtin command line to the >> command >> + * line provided by the bootloader. Kconfig options can be used to alter >> + * the behavior of this builtin command line. >> + * @dst: The destination of the final appended/prepended string. >> + * @src: The starting string or NULL if there isn't one. >> + * @len: the length of dest buffer. >> + */ > > Append or prepend ? Cisco requires both at the same time. This is why my > implementation provides both. I can't use this with both at once. Just an idea: what about defining CMDLINE as a pattern where e.g. "$$" or "%%" is replaced by the boot loader command line? Then you can formulate replace, prepend, append, prepend+append with a single CONFIG setting. It may be a little more complex in code (scanning for the pattern and replacing it and take care to temporary memory) but IMHO it could be worth to consider. BR, Nikolaus Schaller
[PATCH] drm: panel-simple: fix missing DRM_MODE_CONNECTOR_DPI for Ortustech com37h3m
Without we get an "Specify missing connector_type" warning.
Fixes: ddb8e853dc85 ("drm/panel: panel-simple: validate panel description")
Signed-off-by: H. Nikolaus Schaller
---
drivers/gpu/drm/panel/panel-simple.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 4e2dad314c7953..502b5f1c4fd16c 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -3226,6 +3226,7 @@ static const struct panel_desc ortustech_com37h3m = {
.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE
|
DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
+ .connector_type = DRM_MODE_CONNECTOR_DPI,
};
static const struct drm_display_mode ortustech_com43h4m85ulc_mode = {
--
2.26.2
Q: What ist the standard way to define connector type and bus format with device tree?
No expert available? Let me refine the question: By using the descriptions of panel-common.yaml and panel-timing.yaml it is possible to define a simple-panel by pure DTS means. This timing setup is done by code through: git blame drivers/gpu/drm/panel/panel-simple.c | grep of_get_display_timing 4a1d0dbc8332231 (Sam Ravnborg2020-02-16 19:15:13 +0100 431) ret = of_get_display_timing(np, "panel-timing", timing); 4a1d0dbc8332231 (Sam Ravnborg2020-02-16 19:15:13 +0100 566) if (!of_get_display_timing(dev->of_node, "panel-timing", )) But it seems to lack a mechanism to define the connector type and bus format which is needed to make a panel finally work. Are we missing something or is the code/property missing? If it is the first, please let us know so that we can use it. If code is missing, please let us know so that we can suggest a patch. BR and thanks, Nikolaus > Am 31.01.2021 um 11:54 schrieb H. Nikolaus Schaller : > > ping? > >> Am 12.01.2021 um 12:41 schrieb H. Nikolaus Schaller : >> >> Hi, >> according to bindings/display/panel/panel-common.yaml >> and by using "panel-simple" as compatible string we >> can define almost all properties of a DSI panel by a >> device tree entry. >> >> Except the connector type and bus format which can only be >> set by adding the information to the panel-simple tables. >> >> This leads to big problems because DRM can't match the >> display with any lcd controller. A smaller issue is a >> warning: >> >> [0.313431] panel-simple claa070vc01: Specify missing connector_type >> >> Are we missing some documentation or code that reads >> some "connector-type" and "bus-format" property? >> >> BR and thanks, >> Nikolaus >
Re: BOG: commit 89c7cb1608ac3 ("of/device: Update dma_range_map only when dev has valid dma-ranges") seems to break Pinephone display or LCDC
Hi Paul, > Am 02.02.2021 um 10:56 schrieb Paul Kocialkowski > : > > Hi Nikolaus, > > On Tue 02 Feb 21, 10:18, H. Nikolaus Schaller wrote: >> Hi, >> since v5.11-rc6 my Pinephone display shows some moiré pattern. >> >> I did a bisect between v5.11-rc5 and v5.11-rc6 and it told me that >> the commit mentioned in the subject is the reason. >> >> Reverting it makes the display work again and re-reverting fail again. >> >> IMHO it seems as if the display DMA of the pinephone (allwinner suni-a54) >> got influenced and stopped to scan the framebuffer. >> >> The only dma-ranges I could find are defined here: >> >> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> >> dma-ranges = <0x 0x4000 0xc000>; >> >> but I can't tell if they are "valid" or not. >> >> Any insights are welcome. And please direct to the right people/mailing lists >> if they are missing. > > This may not be strictly the same thing, but is this patch in your tree: > https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210115175831.1184260-2-paul.kocialkow...@bootlin.com/ > > If not, it's worth a try to add it. No, it hasn't arrived in v5.11-rc6 (or linux-next) yet. But it fixes the issue. great and many thanks, Nikolaus > If it is, it's worth doing a revert. > > My understanding is like DE2 does not need a particular DMA range and has DRAM > starting at 0x4000 (just like the CPU) but it will map DRAM in a loop > before and after this address. > > I suspect the issue shows because the pinephone has 2 GiB RAM while for other > boards with < 2 GiB RAM, removing 0x4000 to the DMA addresses still points > to the same location. So IMO the MBUS dma-ranges shouldn't apply to DE2. > I think this is already the case in dt, but the mbus driver may add it if you > don't have that patch. > > I think I have a few A64 boards around, but probably not with 2 GiB RAM. > If adding the patch doesn't help, I'll try to make a few test. > > Cheers! > > Paul > > -- > Paul Kocialkowski, Bootlin > Embedded Linux and kernel engineering > https://bootlin.com
BOG: commit 89c7cb1608ac3 ("of/device: Update dma_range_map only when dev has valid dma-ranges") seems to break Pinephone display or LCDC
Hi, since v5.11-rc6 my Pinephone display shows some moiré pattern. I did a bisect between v5.11-rc5 and v5.11-rc6 and it told me that the commit mentioned in the subject is the reason. Reverting it makes the display work again and re-reverting fail again. IMHO it seems as if the display DMA of the pinephone (allwinner suni-a54) got influenced and stopped to scan the framebuffer. The only dma-ranges I could find are defined here: arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi dma-ranges = <0x 0x4000 0xc000>; but I can't tell if they are "valid" or not. Any insights are welcome. And please direct to the right people/mailing lists if they are missing. BR and thanks, Nikolaus Schaller
Re: [Letux-kernel] What ist the standard way to define connector type and bus format with device tree?
ping? > Am 12.01.2021 um 12:41 schrieb H. Nikolaus Schaller : > > Hi, > according to bindings/display/panel/panel-common.yaml > and by using "panel-simple" as compatible string we > can define almost all properties of a DSI panel by a > device tree entry. > > Except the connector type and bus format which can only be > set by adding the information to the panel-simple tables. > > This leads to big problems because DRM can't match the > display with any lcd controller. A smaller issue is a > warning: > > [0.313431] panel-simple claa070vc01: Specify missing connector_type > > Are we missing some documentation or code that reads > some "connector-type" and "bus-format" property? > > BR and thanks, > Nikolaus
Re: [PATCH v3 4/4] drm/ingenic: Fix non-OSD mode
Hi Paul,
> Am 24.01.2021 um 10:47 schrieb H. Nikolaus Schaller :
>
> Hi Paul,
>
>> Am 24.01.2021 um 10:43 schrieb Paul Cercueil :
>>
>> Hi Nikolaus,
>>
>> Le dim. 24 janv. 2021 à 10:30, H. Nikolaus Schaller a
>> écrit :
>>> Hi Paul,
>>> we observed the same issue on the jz4730 (which is almost identical
>>> to the jz4740 wrt. LCDC) and our solution [1] was simpler.
>>> It leaves the hwdesc f0 and f1 as they are and just takes f1 for really
>>> programming the first DMA descriptor if there is no OSD.
>>
>> Disagreed. With your solution, it ends up using priv->f1 plane with
>> hwdesc_f0. That's very confusing.
>
> It is a tradeoff between code simplicity and confusion. Indeed difficult to
> decide. Fortunately I don't have to :)
>
>>
>>> We have tested on jz4730 and jz4780.
>>
>> Could I get a tested-by then? :)
>
> I'll test with our tree and test both SoC in the next days and give feedback.
Ok, works on both devices:
a) Skytone Alpha 400 compatible Letux 400 with not-yet-upstream jz4730 and
internal panel
b) CI20 with external HDMI (tablet) and our not-yet-upstream HDMI code
So the hwdesc handling is as ok as it was with our patch. I'll keep a copy of
yours in our
tree until it arrives upstream.
Tested-by: H. Nikolaus Schaller
BR and thanks,
Nikolaus
>
> BR and thanks,
> Nikolaus
>
>>
>> Cheers,
>> -Paul
>>
>>> Maybe you want to consider that. Then I can officially post it.
>>> [1]
>>> https://github.com/goldelico/letux-kernel/commit/3be1de5fdabf2cc1c17f198ded3328cc6e4b9844
>>>> Am 24.01.2021 um 09:55 schrieb Paul Cercueil :
>>>> Even though the JZ4740 did not have the OSD mode, it had (according to
>>>> the documentation) two DMA channels, but there is absolutely no
>>>> information about how to select the second DMA channel.
>>>> Make the ingenic-drm driver work in non-OSD mode by using the
>>>> foreground0 plane (which is bound to the DMA0 channel) as the primary
>>>> plane, instead of the foreground1 plane, which is the primary plane
>>>> when in OSD mode.
>>>> Fixes: 3c9bea4ef32b ("drm/ingenic: Add support for OSD mode")
>>>> Cc: # v5.8+
>>>> Signed-off-by: Paul Cercueil
>>>> Acked-by: Daniel Vetter
>>>> ---
>>>> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 11 +++
>>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>>> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>>> index b23011c1c5d9..59ce43862e16 100644
>>>> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>>> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>>> @@ -554,7 +554,7 @@ static void ingenic_drm_plane_atomic_update(struct
>>>> drm_plane *plane,
>>>>height = state->src_h >> 16;
>>>>cpp = state->fb->format->cpp[0];
>>>> - if (priv->soc_info->has_osd && plane->type ==
>>>> DRM_PLANE_TYPE_OVERLAY)
>>>> + if (!priv->soc_info->has_osd || plane->type ==
>>>> DRM_PLANE_TYPE_OVERLAY)
>>>>hwdesc = >dma_hwdescs->hwdesc_f0;
>>>>else
>>>>hwdesc = >dma_hwdescs->hwdesc_f1;
>>> we just replace this with
>>> if (priv->soc_info->has_osd && plane->type !=
>>> DRM_PLANE_TYPE_OVERLAY)
>>> hwdesc = >dma_hwdescs->hwdesc_f1;
>>> else
>>> hwdesc = >dma_hwdescs->hwdesc_f0;
>>> and the remainder can stay as is.
>>>> @@ -826,6 +826,7 @@ static int ingenic_drm_bind(struct device *dev, bool
>>>> has_components)
>>>>const struct jz_soc_info *soc_info;
>>>>struct ingenic_drm *priv;
>>>>struct clk *parent_clk;
>>>> + struct drm_plane *primary;
>>>>struct drm_bridge *bridge;
>>>>struct drm_panel *panel;
>>>>struct drm_encoder *encoder;
>>>> @@ -940,9 +941,11 @@ static int ingenic_drm_bind(struct device *dev, bool
>>>> has_components)
>>>>if (soc_info->has_osd)
>>>>priv->ipu_plane = drm_plane_from_index(drm, 0);
>>>> - drm_plane_helper_add(>f1, _drm_plane_helper_funcs);
>>>> + primary = priv->soc_info->has_osd ? >f1 : >f0;
>>>> - ret = drm_universal_plane_init(drm, >f1, 1,
>>>> + drm_plane_helper_add(primary, _drm_plane_helper_funcs);
>>>> +
>>>> + ret = drm_universal_plane_init(drm, primary, 1,
>>>> _drm_primary_plane_funcs,
>>>> priv->soc_info->formats_f1,
>>>> priv->soc_info->num_formats_f1,
>>>> @@ -954,7 +957,7 @@ static int ingenic_drm_bind(struct device *dev, bool
>>>> has_components)
>>>>drm_crtc_helper_add(>crtc, _drm_crtc_helper_funcs);
>>>> - ret = drm_crtc_init_with_planes(drm, >crtc, >f1,
>>>> + ret = drm_crtc_init_with_planes(drm, >crtc, primary,
>>>>NULL, _drm_crtc_funcs, NULL);
>>>>if (ret) {
>>>>dev_err(dev, "Failed to init CRTC: %i\n", ret);
>>>> --
>>>> 2.29.2
>>> BR and thanks,
>>> Nikolaus
>>
>>
>
Re: [PATCH v3 4/4] drm/ingenic: Fix non-OSD mode
Hi Paul,
> Am 24.01.2021 um 10:43 schrieb Paul Cercueil :
>
> Hi Nikolaus,
>
> Le dim. 24 janv. 2021 à 10:30, H. Nikolaus Schaller a
> écrit :
>> Hi Paul,
>> we observed the same issue on the jz4730 (which is almost identical
>> to the jz4740 wrt. LCDC) and our solution [1] was simpler.
>> It leaves the hwdesc f0 and f1 as they are and just takes f1 for really
>> programming the first DMA descriptor if there is no OSD.
>
> Disagreed. With your solution, it ends up using priv->f1 plane with
> hwdesc_f0. That's very confusing.
It is a tradeoff between code simplicity and confusion. Indeed difficult to
decide. Fortunately I don't have to :)
>
>> We have tested on jz4730 and jz4780.
>
> Could I get a tested-by then? :)
I'll test with our tree and test both SoC in the next days and give feedback.
BR and thanks,
Nikolaus
>
> Cheers,
> -Paul
>
>> Maybe you want to consider that. Then I can officially post it.
>> [1]
>> https://github.com/goldelico/letux-kernel/commit/3be1de5fdabf2cc1c17f198ded3328cc6e4b9844
>>> Am 24.01.2021 um 09:55 schrieb Paul Cercueil :
>>> Even though the JZ4740 did not have the OSD mode, it had (according to
>>> the documentation) two DMA channels, but there is absolutely no
>>> information about how to select the second DMA channel.
>>> Make the ingenic-drm driver work in non-OSD mode by using the
>>> foreground0 plane (which is bound to the DMA0 channel) as the primary
>>> plane, instead of the foreground1 plane, which is the primary plane
>>> when in OSD mode.
>>> Fixes: 3c9bea4ef32b ("drm/ingenic: Add support for OSD mode")
>>> Cc: # v5.8+
>>> Signed-off-by: Paul Cercueil
>>> Acked-by: Daniel Vetter
>>> ---
>>> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 11 +++
>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>> index b23011c1c5d9..59ce43862e16 100644
>>> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>> @@ -554,7 +554,7 @@ static void ingenic_drm_plane_atomic_update(struct
>>> drm_plane *plane,
>>> height = state->src_h >> 16;
>>> cpp = state->fb->format->cpp[0];
>>> - if (priv->soc_info->has_osd && plane->type ==
>>> DRM_PLANE_TYPE_OVERLAY)
>>> + if (!priv->soc_info->has_osd || plane->type ==
>>> DRM_PLANE_TYPE_OVERLAY)
>>> hwdesc = >dma_hwdescs->hwdesc_f0;
>>> else
>>> hwdesc = >dma_hwdescs->hwdesc_f1;
>> we just replace this with
>>if (priv->soc_info->has_osd && plane->type !=
>> DRM_PLANE_TYPE_OVERLAY)
>>hwdesc = >dma_hwdescs->hwdesc_f1;
>>else
>>hwdesc = >dma_hwdescs->hwdesc_f0;
>> and the remainder can stay as is.
>>> @@ -826,6 +826,7 @@ static int ingenic_drm_bind(struct device *dev, bool
>>> has_components)
>>> const struct jz_soc_info *soc_info;
>>> struct ingenic_drm *priv;
>>> struct clk *parent_clk;
>>> + struct drm_plane *primary;
>>> struct drm_bridge *bridge;
>>> struct drm_panel *panel;
>>> struct drm_encoder *encoder;
>>> @@ -940,9 +941,11 @@ static int ingenic_drm_bind(struct device *dev, bool
>>> has_components)
>>> if (soc_info->has_osd)
>>> priv->ipu_plane = drm_plane_from_index(drm, 0);
>>> - drm_plane_helper_add(>f1, _drm_plane_helper_funcs);
>>> + primary = priv->soc_info->has_osd ? >f1 : >f0;
>>> - ret = drm_universal_plane_init(drm, >f1, 1,
>>> + drm_plane_helper_add(primary, _drm_plane_helper_funcs);
>>> +
>>> + ret = drm_universal_plane_init(drm, primary, 1,
>>>_drm_primary_plane_funcs,
>>>priv->soc_info->formats_f1,
>>>priv->soc_info->num_formats_f1,
>>> @@ -954,7 +957,7 @@ static int ingenic_drm_bind(struct device *dev, bool
>>> has_components)
>>> drm_crtc_helper_add(>crtc, _drm_crtc_helper_funcs);
>>> - ret = drm_crtc_init_with_planes(drm, >crtc, >f1,
>>> + ret = drm_crtc_init_with_planes(drm, >crtc, primary,
>>> NULL, _drm_crtc_funcs, NULL);
>>> if (ret) {
>>> dev_err(dev, "Failed to init CRTC: %i\n", ret);
>>> --
>>> 2.29.2
>> BR and thanks,
>> Nikolaus
>
>
Re: [PATCH v3 4/4] drm/ingenic: Fix non-OSD mode
Hi Paul,
we observed the same issue on the jz4730 (which is almost identical
to the jz4740 wrt. LCDC) and our solution [1] was simpler.
It leaves the hwdesc f0 and f1 as they are and just takes f1 for really
programming the first DMA descriptor if there is no OSD.
We have tested on jz4730 and jz4780.
Maybe you want to consider that. Then I can officially post it.
[1]
https://github.com/goldelico/letux-kernel/commit/3be1de5fdabf2cc1c17f198ded3328cc6e4b9844
> Am 24.01.2021 um 09:55 schrieb Paul Cercueil :
>
> Even though the JZ4740 did not have the OSD mode, it had (according to
> the documentation) two DMA channels, but there is absolutely no
> information about how to select the second DMA channel.
>
> Make the ingenic-drm driver work in non-OSD mode by using the
> foreground0 plane (which is bound to the DMA0 channel) as the primary
> plane, instead of the foreground1 plane, which is the primary plane
> when in OSD mode.
>
> Fixes: 3c9bea4ef32b ("drm/ingenic: Add support for OSD mode")
> Cc: # v5.8+
> Signed-off-by: Paul Cercueil
> Acked-by: Daniel Vetter
> ---
> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index b23011c1c5d9..59ce43862e16 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -554,7 +554,7 @@ static void ingenic_drm_plane_atomic_update(struct
> drm_plane *plane,
> height = state->src_h >> 16;
> cpp = state->fb->format->cpp[0];
>
> - if (priv->soc_info->has_osd && plane->type ==
> DRM_PLANE_TYPE_OVERLAY)
> + if (!priv->soc_info->has_osd || plane->type ==
> DRM_PLANE_TYPE_OVERLAY)
> hwdesc = >dma_hwdescs->hwdesc_f0;
> else
> hwdesc = >dma_hwdescs->hwdesc_f1;
we just replace this with
if (priv->soc_info->has_osd && plane->type !=
DRM_PLANE_TYPE_OVERLAY)
hwdesc = >dma_hwdescs->hwdesc_f1;
else
hwdesc = >dma_hwdescs->hwdesc_f0;
and the remainder can stay as is.
> @@ -826,6 +826,7 @@ static int ingenic_drm_bind(struct device *dev, bool
> has_components)
> const struct jz_soc_info *soc_info;
> struct ingenic_drm *priv;
> struct clk *parent_clk;
> + struct drm_plane *primary;
> struct drm_bridge *bridge;
> struct drm_panel *panel;
> struct drm_encoder *encoder;
> @@ -940,9 +941,11 @@ static int ingenic_drm_bind(struct device *dev, bool
> has_components)
> if (soc_info->has_osd)
> priv->ipu_plane = drm_plane_from_index(drm, 0);
>
> - drm_plane_helper_add(>f1, _drm_plane_helper_funcs);
> + primary = priv->soc_info->has_osd ? >f1 : >f0;
>
> - ret = drm_universal_plane_init(drm, >f1, 1,
> + drm_plane_helper_add(primary, _drm_plane_helper_funcs);
> +
> + ret = drm_universal_plane_init(drm, primary, 1,
> _drm_primary_plane_funcs,
> priv->soc_info->formats_f1,
> priv->soc_info->num_formats_f1,
> @@ -954,7 +957,7 @@ static int ingenic_drm_bind(struct device *dev, bool
> has_components)
>
> drm_crtc_helper_add(>crtc, _drm_crtc_helper_funcs);
>
> - ret = drm_crtc_init_with_planes(drm, >crtc, >f1,
> + ret = drm_crtc_init_with_planes(drm, >crtc, primary,
> NULL, _drm_crtc_funcs, NULL);
> if (ret) {
> dev_err(dev, "Failed to init CRTC: %i\n", ret);
> --
> 2.29.2
>
BR and thanks,
Nikolaus
Re: [PATCH] pinctrl: ingenic: Improve JZ4760 support
> Am 20.01.2021 um 12:07 schrieb Paul Cercueil :
>
> - Add otg function and otg-vbus group.
>
> - Add lcd-8bit, lcd-16bit, lcd-18bit, lcd-generic and lcd-special
> groups. Change the lcd-24bit group so that it only selects the pins
> that aren't in the lcd-18bit and lcd-generic groups (which breaks
> Device Tree in theory, but there is none out there for any JZ4760
> based board, yet). Remove the lcd-no-pins group which is just useless.
Does this mean it is also usless for the other Ingenic SoC?
Background: we are working on jz4730 support and have simply copied it.
>
> Signed-off-by: Paul Cercueil
> ---
> drivers/pinctrl/pinctrl-ingenic.c | 38 +++
> 1 file changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-ingenic.c
> b/drivers/pinctrl/pinctrl-ingenic.c
> index 76fec77c5b67..f2746125b077 100644
> --- a/drivers/pinctrl/pinctrl-ingenic.c
> +++ b/drivers/pinctrl/pinctrl-ingenic.c
> @@ -376,12 +376,21 @@ static int jz4760_cim_pins[] = {
> 0x26, 0x27, 0x28, 0x29,
> 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f, 0x30, 0x31,
> };
> +static int jz4760_lcd_8bit_pins[] = {
> + 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, 0x4c,
> + 0x4d, 0x52, 0x53,
> +};
> +static int jz4760_lcd_16bit_pins[] = {
> + 0x4e, 0x4f, 0x50, 0x51, 0x56, 0x57, 0x58, 0x59,
> +};
> +static int jz4760_lcd_18bit_pins[] = {
> + 0x5a, 0x5b,
> +};
> static int jz4760_lcd_24bit_pins[] = {
> - 0x40, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47,
> - 0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f,
> - 0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57,
> - 0x58, 0x59, 0x5a, 0x5b,
> + 0x40, 0x41, 0x4a, 0x4b, 0x54, 0x55,
> };
> +static int jz4760_lcd_special_pins[] = { 0x40, 0x41, 0x4a, 0x54 };
> +static int jz4760_lcd_generic_pins[] = { 0x49, };
> static int jz4760_pwm_pwm0_pins[] = { 0x80, };
> static int jz4760_pwm_pwm1_pins[] = { 0x81, };
> static int jz4760_pwm_pwm2_pins[] = { 0x82, };
> @@ -390,6 +399,7 @@ static int jz4760_pwm_pwm4_pins[] = { 0x84, };
> static int jz4760_pwm_pwm5_pins[] = { 0x85, };
> static int jz4760_pwm_pwm6_pins[] = { 0x6a, };
> static int jz4760_pwm_pwm7_pins[] = { 0x6b, };
> +static int jz4760_otg_pins[] = { 0x8a, };
>
> static u8 jz4760_uart3_data_funcs[] = { 0, 1, };
> static u8 jz4760_mmc0_1bit_a_funcs[] = { 1, 1, 0, };
> @@ -436,8 +446,12 @@ static const struct group_desc jz4760_groups[] = {
> INGENIC_PIN_GROUP("i2c0-data", jz4760_i2c0, 0),
> INGENIC_PIN_GROUP("i2c1-data", jz4760_i2c1, 0),
> INGENIC_PIN_GROUP("cim-data", jz4760_cim, 0),
> + INGENIC_PIN_GROUP("lcd-8bit", jz4760_lcd_8bit, 0),
> + INGENIC_PIN_GROUP("lcd-16bit", jz4760_lcd_16bit, 0),
> + INGENIC_PIN_GROUP("lcd-18bit", jz4760_lcd_18bit, 0),
> INGENIC_PIN_GROUP("lcd-24bit", jz4760_lcd_24bit, 0),
> - { "lcd-no-pins", },
> + INGENIC_PIN_GROUP("lcd-generic", jz4760_lcd_generic, 0),
> + INGENIC_PIN_GROUP("lcd-special", jz4760_lcd_special, 1),
> INGENIC_PIN_GROUP("pwm0", jz4760_pwm_pwm0, 0),
> INGENIC_PIN_GROUP("pwm1", jz4760_pwm_pwm1, 0),
> INGENIC_PIN_GROUP("pwm2", jz4760_pwm_pwm2, 0),
> @@ -446,6 +460,7 @@ static const struct group_desc jz4760_groups[] = {
> INGENIC_PIN_GROUP("pwm5", jz4760_pwm_pwm5, 0),
> INGENIC_PIN_GROUP("pwm6", jz4760_pwm_pwm6, 0),
> INGENIC_PIN_GROUP("pwm7", jz4760_pwm_pwm7, 0),
> + INGENIC_PIN_GROUP("otg-vbus", jz4760_otg, 0),
> };
>
> static const char *jz4760_uart0_groups[] = { "uart0-data", "uart0-hwflow", };
> @@ -477,7 +492,10 @@ static const char *jz4760_cs6_groups[] = { "nemc-cs6", };
> static const char *jz4760_i2c0_groups[] = { "i2c0-data", };
> static const char *jz4760_i2c1_groups[] = { "i2c1-data", };
> static const char *jz4760_cim_groups[] = { "cim-data", };
> -static const char *jz4760_lcd_groups[] = { "lcd-24bit", "lcd-no-pins", };
> +static const char *jz4760_lcd_groups[] = {
> + "lcd-8bit", "lcd-16bit", "lcd-18bit", "lcd-24bit",
> + "lcd-special", "lcd-generic",
> +};
> static const char *jz4760_pwm0_groups[] = { "pwm0", };
> static const char *jz4760_pwm1_groups[] = { "pwm1", };
> static const char *jz4760_pwm2_groups[] = { "pwm2", };
> @@ -486,6 +504,7 @@ static const char *jz4760_pwm4_groups[] = { "pwm4", };
> static const char *jz4760_pwm5_groups[] = { "pwm5", };
> static const char *jz4760_pwm6_groups[] = { "pwm6", };
> static const char *jz4760_pwm7_groups[] = { "pwm7", };
> +static const char *jz4760_otg_groups[] = { "otg-vbus", };
>
> static const struct function_desc jz4760_functions[] = {
> { "uart0", jz4760_uart0_groups, ARRAY_SIZE(jz4760_uart0_groups), },
> @@ -514,6 +533,7 @@ static const struct function_desc jz4760_functions[] = {
> { "pwm5", jz4760_pwm5_groups, ARRAY_SIZE(jz4760_pwm5_groups), },
> { "pwm6", jz4760_pwm6_groups, ARRAY_SIZE(jz4760_pwm6_groups), },
> { "pwm7", jz4760_pwm7_groups, ARRAY_SIZE(jz4760_pwm7_groups), },
> + { "otg", jz4760_otg_groups,
Re: [Letux-kernel] [PATCH 3/4] ARM: dts: imx6sl-tolino-shine3: correct console uart pinmux
Ah, there is a v2. Let's hope this gets merged/processed by robots correctly :)
> Am 13.01.2021 um 00:15 schrieb Andreas Kemnade :
>
> Configuration was correct enough to work with the pre-configuration done by
> uboot. While at it, also document the location.
>
> Signed-off-by: Andreas Kemnade
> ---
> arch/arm/boot/dts/imx6sl-tolino-shine3.dts | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> b/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> index 27143ea0f0f1..62d2ebda04ff 100644
> --- a/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> +++ b/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> @@ -156,7 +156,7 @@
> pinctrl_uart1: uart1grp {
> fsl,pins = <
> MX6SL_PAD_UART1_TXD__UART1_TX_DATA 0x1b0b1
> - MX6SL_PAD_UART1_RXD__UART1_TX_DATA 0x1b0b1
> + MX6SL_PAD_UART1_RXD__UART1_RX_DATA 0x1b0b1
> >;
> };
>
> --
> 2.20.1
>
> ___
> https://projects.goldelico.com/p/gta04-kernel/
> Letux-kernel mailing list
> letux-ker...@openphoenux.org
> http://lists.goldelico.com/mailman/listinfo.cgi/letux-kernel
Re: [Letux-kernel] [PATCH 3/4] ARM: dts: imx6sl-tolino-shine2hd: correct console uart pinmux
> Am 13.01.2021 um 00:15 schrieb Andreas Kemnade :
>
> Configuration was correct enough to work with the pre-configuration done by
> uboot. While at it, also document the location.
>
> Signed-off-by: Andreas Kemnade
> ---
> arch/arm/boot/dts/imx6sl-tolino-shine3.dts | 2 +-
seems to have a typo (shine2hd vs. shine3) in the patch subject.
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> b/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> index 27143ea0f0f1..62d2ebda04ff 100644
> --- a/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> +++ b/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> @@ -156,7 +156,7 @@
> pinctrl_uart1: uart1grp {
> fsl,pins = <
> MX6SL_PAD_UART1_TXD__UART1_TX_DATA 0x1b0b1
> - MX6SL_PAD_UART1_RXD__UART1_TX_DATA 0x1b0b1
> + MX6SL_PAD_UART1_RXD__UART1_RX_DATA 0x1b0b1
> >;
> };
>
> --
> 2.20.1
>
> ___
> https://projects.goldelico.com/p/gta04-kernel/
> Letux-kernel mailing list
> letux-ker...@openphoenux.org
> http://lists.goldelico.com/mailman/listinfo.cgi/letux-kernel
What ist the standard way to define connector type and bus format with device tree?
Hi, according to bindings/display/panel/panel-common.yaml and by using "panel-simple" as compatible string we can define almost all properties of a DSI panel by a device tree entry. Except the connector and bus format which can only be set by adding the same information to the panel-simple tables. This leads to big problems because DRM can't match the display with any lcd controller. The least issue is a warning: [0.313431] panel-simple claa070vc01: Specify missing connector_type Are we missing some documentation or code that reads some "connector-type" and "bus-format" property? BR and thanks, Nikolaus
Re: [patch V3 13/37] mips/mm/highmem: Switch to generic kmap atomic
> Am 10.01.2021 um 12:35 schrieb Paul Cercueil : > > Hi Thomas, > > Le sam. 9 janv. 2021 à 1:33, Thomas Bogendoerfer > a écrit : >> On Sat, Jan 09, 2021 at 12:58:05AM +0100, Thomas Bogendoerfer wrote: >>> On Fri, Jan 08, 2021 at 08:20:43PM +, Paul Cercueil wrote: >>> > Hi Thomas, >>> > >>> > 5.11 does not boot anymore on Ingenic SoCs, I bisected it to this commit. Just for completeness, I have no such problems booting CI20/jz4780 or Skytone400/jz4730 (unpublished work) with 5.11-rc2. But may depend on board capabilites (ram size, memory layout or something else). >>> > >>> > Any idea what could be happening? >>> not yet, kernel crash log of a Malta QEMU is below. >> update: >> This dirty hack lets the Malta QEMU boot again: >> diff --git a/mm/highmem.c b/mm/highmem.c >> index c3a9ea7875ef..190cdda1149d 100644 >> --- a/mm/highmem.c >> +++ b/mm/highmem.c >> @@ -515,7 +515,7 @@ void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t >> prot) >> vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx); >> BUG_ON(!pte_none(*(kmap_pte - idx))); >> pteval = pfn_pte(pfn, prot); >> -set_pte_at(_mm, vaddr, kmap_pte - idx, pteval); >> +set_pte(kmap_pte - idx, pteval); >> arch_kmap_local_post_map(vaddr, pteval); >> current->kmap_ctrl.pteval[kmap_local_idx()] = pteval; >> preempt_enable(); >> set_pte_at() tries to update cache and could do an kmap_atomic() there. >> Not sure, if this is allowed at this point. > > Yes, I can confirm that your workaround works here too. > > Cheers, > -Paul > >
Re: [PATCH] ARM: dts: omap36xx: Remove turbo mode for 1GHz variants
Hi Adam,
> Am 09.01.2021 um 17:39 schrieb Adam Ford :
>
> Previously, the 1GHz variants were marked as a turbo,
> because that variant has reduced thermal operating range.
>
> Now that the thermal throttling is in place, it should be
> safe to remove the turbo-mode from the 1GHz variants, because
> the CPU will automatically slow if the thermal limit is reached.
Subject and description may be misunderstood in a way that 1GHz
is now disabled.
Rather the 1GHz OPP is now permanently enabled and does no longer
need to be manuall enabled through something like
/sys/devices/system/cpu/cpufreq/boost.
>
> Signed-off-by: Adam Ford
>
> diff --git a/arch/arm/boot/dts/logicpd-torpedo-som.dtsi
> b/arch/arm/boot/dts/logicpd-torpedo-som.dtsi
> index 3a5228562b0d..3451f9be104e 100644
> --- a/arch/arm/boot/dts/logicpd-torpedo-som.dtsi
> +++ b/arch/arm/boot/dts/logicpd-torpedo-som.dtsi
> @@ -70,6 +70,7 @@ nand@0,0 {
> gpmc,device-width = <2>;
> #address-cells = <1>;
> #size-cells = <1>;
> + status = "disabled";
this does not seem to match the description?
> };
> };
>
> --
> 2.25.1
>
BR,
Nikolaus
Re: [PATCH 1/3] thermal: ti-soc-thermal: Fix stuck sensor with continuous mode for 4430
Hi Adam and Tony,
> Am 30.12.2020 um 13:55 schrieb Adam Ford :
>
> On Wed, Dec 30, 2020 at 2:43 AM Tony Lindgren wrote:
>>
>> At least for 4430, trying to use the single conversion mode eventually
>> hangs the thermal sensor. This can be quite easily seen with errors:
>>
>> thermal thermal_zone0: failed to read out thermal zone (-5)
>>
>> Also, trying to read the temperature shows a stuck value with:
>>
>> $ while true; do cat /sys/class/thermal/thermal_zone0/temp; done
>>
>> Where the temperature is not rising at all with the busy loop.
>>
>> Additionally, the EOCZ (end of conversion) bit is not rising on 4430 in
>> single conversion mode while it works fine in continuous conversion mode.
>> It is also possible that the hung temperature sensor can affect the
>> thermal shutdown alert too.
>>
>> Let's fix the issue by adding TI_BANDGAP_FEATURE_CONT_MODE_ONLY flag and
>> use it for 4430.
>>
>> Note that we also need to add udelay to for the EOCZ (end of conversion)
>> bit polling as otherwise we have it time out too early on 4430. We'll be
>> changing the loop to use iopoll in the following clean-up patch.
>>
>> Cc: Adam Ford
>
> I don't have an OMAP4, but if you want, I can test a DM3730.
Indeed I remember a similar discussion from the DM3730 [1]. temp values were
always those from the last measurement. E.g. the first one was done
during (cold) boot and the first request after 10 minutes did show a
quite cold system... The next one did show a hot system independent
of what had been between (suspend or high activity).
It seems as if it was even reproducible with a very old kernel on a BeagleBoard.
So it is quite fundamental.
We tried to fix it but did not come to a solution [2]. So we opened an issue
in our tracker [3] and decided to stay with continuous conversion although this
raises idle mode processor load.
BR,
Nikolaus
[1]:
https://lists.goldelico.com/pipermail/letux-kernel/2019-September/003958.html
[2]:
https://lists.goldelico.com/pipermail/letux-kernel/2019-September/003975.html
[3]: https://projects.goldelico.com/p/gta04-kernel/issues/928/
> adam
>
>> Cc: Carl Philipp Klemm
>> Cc: Eduardo Valentin
>> Cc: Merlijn Wajer
>> Cc: Pavel Machek
>> Cc: Peter Ujfalusi
>> Cc: Sebastian Reichel
>> Signed-off-by: Tony Lindgren
>> ---
>> drivers/thermal/ti-soc-thermal/omap4-thermal-data.c | 3 ++-
>> drivers/thermal/ti-soc-thermal/ti-bandgap.c | 9 +++--
>> drivers/thermal/ti-soc-thermal/ti-bandgap.h | 2 ++
>> 3 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/thermal/ti-soc-thermal/omap4-thermal-data.c
>> b/drivers/thermal/ti-soc-thermal/omap4-thermal-data.c
>> --- a/drivers/thermal/ti-soc-thermal/omap4-thermal-data.c
>> +++ b/drivers/thermal/ti-soc-thermal/omap4-thermal-data.c
>> @@ -58,7 +58,8 @@ omap4430_adc_to_temp[OMAP4430_ADC_END_VALUE -
>> OMAP4430_ADC_START_VALUE + 1] = {
>> const struct ti_bandgap_data omap4430_data = {
>>.features = TI_BANDGAP_FEATURE_MODE_CONFIG |
>>TI_BANDGAP_FEATURE_CLK_CTRL |
>> - TI_BANDGAP_FEATURE_POWER_SWITCH,
>> + TI_BANDGAP_FEATURE_POWER_SWITCH |
>> + TI_BANDGAP_FEATURE_CONT_MODE_ONLY,
>>.fclock_name = "bandgap_fclk",
>>.div_ck_name = "bandgap_fclk",
>>.conv_table = omap4430_adc_to_temp,
>> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
>> b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
>> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
>> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
>> @@ -15,6 +15,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -605,8 +606,10 @@ ti_bandgap_force_single_read(struct ti_bandgap *bgp,
>> int id)
>>u32 counter = 1000;
>>struct temp_sensor_registers *tsr;
>>
>> - /* Select single conversion mode */
>> - if (TI_BANDGAP_HAS(bgp, MODE_CONFIG))
>> + /* Select continuous or single conversion mode */
>> + if (TI_BANDGAP_HAS(bgp, CONT_MODE_ONLY))
>> + RMW_BITS(bgp, id, bgap_mode_ctrl, mode_ctrl_mask, 1);
>> + else if (TI_BANDGAP_HAS(bgp, MODE_CONFIG))
>>RMW_BITS(bgp, id, bgap_mode_ctrl, mode_ctrl_mask, 0);
>>
>>/* Start of Conversion = 1 */
>> @@ -619,6 +622,7 @@ ti_bandgap_force_single_read(struct ti_bandgap *bgp, int
>> id)
>>if (ti_bandgap_readl(bgp, tsr->temp_sensor_ctrl) &
>>tsr->bgap_eocz_mask)
>>break;
>> + udelay(1);
>>}
>>
>>/* Start of Conversion = 0 */
>> @@ -630,6 +634,7 @@ ti_bandgap_force_single_read(struct ti_bandgap *bgp, int
>> id)
>>if (!(ti_bandgap_readl(bgp, tsr->temp_sensor_ctrl) &
>> tsr->bgap_eocz_mask))
>>break;
>> + udelay(1);
>>}
>>
>>return 0;
>> diff --git
[PATCH] DTS: ARM: gta04: SPI panel chip select is active low
With the arrival of
commit 2fee9583198eb9 ("spi: dt-bindings: clarify CS behavior for spi-cs-high
and gpio descriptors")
it was clarified what the proper state for cs-gpios should be, even if the
flag is ignored. The driver code is doing the right thing since
766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors")
The chip-select of the td028ttec1 panel is active-low, so we must omit
spi-cs-high;
attribute (already removed by separate patch) and should now use
GPIO_ACTIVE_LOW for
the client device description to be fully consistent.
Fixes: 766c6b63aa04 ("spi: fix client driver breakages when using GPIO
descriptors")
CC: sta...@vger.kernel.org
Signed-off-by: H. Nikolaus Schaller
---
arch/arm/boot/dts/omap3-gta04.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi
b/arch/arm/boot/dts/omap3-gta04.dtsi
index 003202d129907b..7b8c18e6605e40 100644
--- a/arch/arm/boot/dts/omap3-gta04.dtsi
+++ b/arch/arm/boot/dts/omap3-gta04.dtsi
@@ -114,7 +114,7 @@ spi_lcd: spi_lcd {
gpio-sck = < 12 GPIO_ACTIVE_HIGH>;
gpio-miso = < 18 GPIO_ACTIVE_HIGH>;
gpio-mosi = < 20 GPIO_ACTIVE_HIGH>;
- cs-gpios = < 19 GPIO_ACTIVE_HIGH>;
+ cs-gpios = < 19 GPIO_ACTIVE_LOW>;
num-chipselects = <1>;
/* lcd panel */
--
2.26.2
[PATCH 0/2] mmc: remove unused struct component card_detect_irq
card_detect_irq is not used anymore since v4.1. Remove it. H. Nikolaus Schaller (2): mmc: jz4740: remove unused struct component card_detect_irq mmc: omap: remove unused struct component card_detect_irq drivers/mmc/host/jz4740_mmc.c | 1 - include/linux/platform_data/mmc-omap.h | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) -- 2.26.2
[PATCH 1/2] mmc: jz4740: remove unused struct component card_detect_irq
I have not found any user for this struct component.
Signed-off-by: H. Nikolaus Schaller
---
drivers/mmc/host/jz4740_mmc.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index a1f92fed2a55b7..b3c636edbb4610 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -152,7 +152,6 @@ struct jz4740_mmc_host {
enum jz4740_mmc_version version;
int irq;
- int card_detect_irq;
void __iomem *base;
struct resource *mem_res;
--
2.26.2
[PATCH 2/2] mmc: omap: remove unused struct component card_detect_irq
I have not found any user for this struct component.
Signed-off-by: H. Nikolaus Schaller
---
include/linux/platform_data/mmc-omap.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/platform_data/mmc-omap.h
b/include/linux/platform_data/mmc-omap.h
index f0b8947e6b07d8..91051e9907f34e 100644
--- a/include/linux/platform_data/mmc-omap.h
+++ b/include/linux/platform_data/mmc-omap.h
@@ -108,8 +108,7 @@ struct omap_mmc_platform_data {
const char *name;
u32 ocr_mask;
- /* Card detection IRQs */
- int card_detect_irq;
+ /* Card detection */
int (*card_detect)(struct device *dev, int slot);
unsigned int ban_openended:1;
--
2.26.2
[PATCH] DTS: ARM: gta04: remove legacy spi-cs-high to make display work again
This reverts
commit f1f028ff89cb ("DTS: ARM: gta04: introduce legacy spi-cs-high to make
display work again")
which had to be intruduced after
commit 6953c57ab172 ("gpio: of: Handle SPI chipselect legacy bindings")
broke the GTA04 display. This contradicted the data sheet but was the only
way to get it as an spi client operational again.
The panel data sheet defines the chip-select to be active low.
Now, with the arrival of
commit 766c6b63aa04 ("spi: fix client driver breakages when using GPIO
descriptors")
the logic of interaction between spi-cs-high and the gpio descriptor flags
has been changed a second time, making the display broken again. So we have
to remove the original fix which in retrospect was a workaround of a bug in
the spi subsystem and not a feature of the panel or bug in the device tree.
With this fix the device tree is back in sync with the data sheet and
spi subsystem code.
Fixes: 766c6b63aa04 ("spi: fix client driver breakages when using GPIO
descriptors")
CC: sta...@vger.kernel.org
Signed-off-by: H. Nikolaus Schaller
---
arch/arm/boot/dts/omap3-gta04.dtsi | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi
b/arch/arm/boot/dts/omap3-gta04.dtsi
index c8745bc800f71..003202d129907 100644
--- a/arch/arm/boot/dts/omap3-gta04.dtsi
+++ b/arch/arm/boot/dts/omap3-gta04.dtsi
@@ -124,7 +124,6 @@ lcd: td028ttec1@0 {
spi-max-frequency = <10>;
spi-cpol;
spi-cpha;
- spi-cs-high;
backlight= <>;
label = "lcd";
--
2.26.2
Re: [PATCH] spi: dt-bindings: clarify CS behavior for spi-cs-high and gpio descriptors
> Am 09.12.2020 um 22:28 schrieb Sven Van Asbroeck :
>
> On Wed, Dec 9, 2020 at 3:08 PM H. Nikolaus Schaller
> wrote:
>>
>> But I have tested with
>>
>>> spi->mode |= SPI_MODE_3;
>>
>> which should keep the mode intact. Right? That did not work either.
>>
>
> - make sure ("spi: fix client driver breakages when using GPIO descriptors")
> is in your tree
Well, if you remember, the panel did work *before* this patch was in my tree
and I found this patch as the reason of the break...
> - your panel's CS is active-low, so 'spi-cs-high' should be removed from its
> devicetree entry. In accordance with the rules as explained in commit
> message of 6953c57ab172. Also in accordance with the table you posted
> in this patch.
It could not have been different because the table was the result of
experimentally checking all possible combinations...
>
> When these two changes in place, your panel should work. I have tested this
> by mirroring your setup on my board:
>
> spi5-gpio {
> compatible = "spi-gpio";
> #address-cells = <0x1>;
> #size-cells = <0x0>;
> pinctrl-names = "default";
> pinctrl-0 = <&...>;
>
> sck-gpios = < GPIO_ACTIVE_HIGH>;
> miso-gpios = < GPIO_ACTIVE_HIGH>;
> mosi-gpios = < GPIO_ACTIVE_HIGH>;
> cs-gpios = < GPIO_ACTIVE_HIGH>;
BTW: exactly this choice is questionable ^^^ if you have an active low CS
and it needs an explanation.
> num-chipselects = <1>;
>
> ethernet-switch@0 { /* active low cs */
> compatible = "micrel,ksz8795";
> spi-max-frequency = <100>;
> reg = <0>;
> };
> };
>
> If this does not work for you, then what are we missing?
I am missing that you notice that we are not discussing what I should
do with the panel driver or my device tree. I have these patches laying around
for a while (which exactly do what you try to convince me about - except that
I would apply an GPIO_ACTIVE_LOW). Just not submitted because I want to
have a clear definition agreed on first. For a simple reason: reviewers
of my patch should know what to check for.
In this thread we discuss a patch for the SPI bindings documentation which
is something different. See subject and the file the patch affects.
And I am looking for an ack and merge by maintainers of the affected subsystems
that the table is ok. Nothing else.
Please let's stay on topic and please cooperate.
Re: [PATCH] spi: dt-bindings: clarify CS behavior for spi-cs-high and gpio descriptors
Hi Andreas, > Am 09.12.2020 um 21:01 schrieb Andreas Kemnade : > > On Wed, 9 Dec 2020 14:04:26 -0500 > Sven Van Asbroeck wrote: > >> On Wed, Dec 9, 2020 at 1:16 PM H. Nikolaus Schaller >> wrote: >>> >>> This is also what made me wonder if that is really intended because then >>> the whole discussion about the cs-gpio-flags and inversion and the fixes >>> would not have been needed. The current code and fixes are all about >>> not ignoring the flags... >> >> The inversion you witnessed was a bug caused by spi client drivers that >> simply "plow over" the SPI_CS_HIGH mode flag. This includes the panel driver >> you're using, see: >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c?h=v5.10-rc6#n337 >> > ah, it would be set in spi->mode and is cleared by > > spi->mode = SPI_MODE_3; > > > Hmm, but we have > spi-cpol; >spi-cpha; > in devicetree. Why do we need that spi->mode line at all? Because it is there in almost all or at least many drivers. But I have tested with > spi->mode |= SPI_MODE_3; which should keep the mode intact. Right? That did not work either. So let's not derail the discussion by moving to the code of some specific driver. Even if that is wrong it does not solve what this patch wants to solve. BR and thanks, Nikolaus
Re: [PATCH] spi: dt-bindings: clarify CS behavior for spi-cs-high and gpio descriptors
> Am 09.12.2020 um 20:04 schrieb Sven Van Asbroeck : > > On Wed, Dec 9, 2020 at 1:16 PM H. Nikolaus Schaller > wrote: >> >> This is also what made me wonder if that is really intended because then >> the whole discussion about the cs-gpio-flags and inversion and the fixes >> would not have been needed. The current code and fixes are all about >> not ignoring the flags... > > The inversion you witnessed was a bug caused by spi client drivers that The inversion we witnessed came from: commit 6953c57ab172 "gpio: of: Handle SPI chipselect legacy bindings" There, I read a verbal description of the table I want to formalize with this patch, because natural language is not as precise as the language of logic. This has nothing to do with driver code, which remained and remains unchanged for long time. > >> Secondly, please imagine some reader of a device tree who finds >> >> cs-gpios = < 7 ACTIVE_LOW>; >> spi-cs-high; > > That reader looks at the rules, sees that: > - the ACTIVE_LOW is ignored, > - presence of spi-cs-high means active-high > and concludes this chip-select is active-high. This misses information what the reader should do to resolve the obviously missing beauty of the DT. a) remove spi-cs-high; b) change to ACTIVE_HIGH Both appear valid in first place. But one is preferred. This is again nowhere documented if you simplify the table.
Re: [PATCH] spi: dt-bindings: clarify CS behavior for spi-cs-high and gpio descriptors
> Am 09.12.2020 um 18:36 schrieb Sven Van Asbroeck :
>
> On Wed, Dec 9, 2020 at 4:57 AM H. Nikolaus Schaller
> wrote:
>>
>> +
>> + device node | cs-gpio | CS pin state active | Note
>> + +===+=+=
>> + spi-cs-high | - | H |
>> + - | - | L |
>> + spi-cs-high | ACTIVE_HIGH | H |
>> + - | ACTIVE_HIGH | L | 1
>> + spi-cs-high | ACTIVE_LOW| H | 2
>> + - | ACTIVE_LOW| L |
>> +
>
> Doesn't this table simply say:
> - specify 'spi-cs-high' for an active-high chip select
> - leave out 'spi-cs-high' for an active-low chip select
> - the gpio active high/active low consumer flags are ignored
> ?
Yes it does, but I don't know if it is what we want to have. Linus confirmed
and you also seem to agree. Let's wait for other verdicts.
This is also what made me wonder if that is really intended because then
the whole discussion about the cs-gpio-flags and inversion and the fixes
would not have been needed. The current code and fixes are all about
not ignoring the flags...
And I am sure the code would be much simpler than currently for treating
special cases. Code would simply be: make any spi driver look at the gpio
descriptor and undo any inversion that gpiod_set_value() will do in
gpiod_set_value_nocheck() so that we can really control the physical
state by spi-cs-high instead of the logical gpio activity.
Something like:
static void spi_gpio_chipselect(struct spi_device *spi, int is_active)
{
struct spi_gpio *spi_gpio = spi_to_spi_gpio(spi);
/* set initial clock line level */
if (is_active)
gpiod_set_value_cansleep(spi_gpio->sck, spi->mode & SPI_CPOL);
/* Drive chip select line, if we have one */
if (spi_gpio->cs_gpios) {
struct gpio_desc *cs = spi_gpio->cs_gpios[spi->chip_select];
/* check if gpiod_set_value_nocheck() will invert */
if (test_bit(FLAG_ACTIVE_LOW, >flags)
is_active = !is_active;
/* SPI chip selects are normally active-low */
gpiod_set_value_cansleep(cs, (spi->mode & SPI_CS_HIGH) ?
is_active : !is_active);
}
}
There would be no need to detect spi-cs-high etc. in gpio-lib or
elsewhere. Only for printing warnings as suggested by Notes 1 and 2.
>
> If so, then I would simply document it that way.
> Simple is beautiful.
Firstly, I would only think about collapsing the table if we agree that
it is correct. Beauty is IMHO not a reason to declare the table to be
correct.
Secondly, please imagine some reader of a device tree who finds
cs-gpios = < 7 ACTIVE_LOW>;
spi-cs-high;
Documentation should work well and be helpful especially in such a case.
Otherwise you don't need documentation.
Saying that the gpio flags are ignored would be helpful but a full table
with Notes and recommendations how to resolve is even more helpful and
unambiguous - even if it tells the same.
BR and thanks,
Nikolaus
[PATCH] spi: dt-bindings: clarify CS behavior for spi-cs-high and gpio descriptors
Behavior of CS signal in combination of spi-cs-high and gpio descriptors is not clearly defined and documented. So clarify the documentation Cc: linus.wall...@linaro.org Cc: linux-g...@vger.kernel.org Signed-off-by: H. Nikolaus Schaller --- .../bindings/spi/spi-controller.yaml | 27 +++ 1 file changed, 27 insertions(+) diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml index 1b56d5e40f1fc..5f505810104dd 100644 --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml @@ -42,6 +42,33 @@ properties: cs2 : 1 0 cs3 : 2 0 + The second flag of a gpio descriptor can be GPIO_ACTIVE_HIGH (0) + or GPIO_ACTIVE_LOW(1). Legacy device trees often use 0. + + There is a special rule set for combining the second flag of an + cs-gpio with the optional spi-cs-high flag for SPI slaves. + + Each table entry defines how the CS pin is to be physically + driven (not considering potential gpio inversions by pinmux): + + device node | cs-gpio | CS pin state active | Note + +===+=+= + spi-cs-high | - | H | + - | - | L | + spi-cs-high | ACTIVE_HIGH | H | + - | ACTIVE_HIGH | L | 1 + spi-cs-high | ACTIVE_LOW| H | 2 + - | ACTIVE_LOW| L | + + Notes: + 1) Should print a warning about polarity inversion. + Here it would be wise to avoid and define the gpio as + ACTIVE_LOW. + 2) Should print a warning about polarity inversion + because ACTIVE_LOW is overridden by spi-cs-high. + Should be generally avoided and be replaced by + spi-cs-high + ACTIVE_HIGH. + num-cs: $ref: /schemas/types.yaml#/definitions/uint32 description: -- 2.26.2
Re: [BUG] SPI broken for SPI based panel drivers
Hi Linus, > Am 09.12.2020 um 09:38 schrieb Linus Walleij : > > On Sat, Dec 5, 2020 at 8:07 AM H. Nikolaus Schaller > wrote: > >> I find it interesting that so far nobody wants to take responsibility >> for a decision > (...) > > >>> What I can do is to provide just a skeleton for the table that you or Linus >>> can fix/fill in and make a patch out of it. Is attached and the ??? is >>> something you should discuss and define. >> >> Please take the attached diff, comment it here and define the question marks >> according to your intention and then make a patch for the YAML bindings out >> of it. (I can't do because I don't know your intentions and what to write >> into >> the commit message). > > I'll comment what I know, then you can send a proper patch to > Mark. But you really need more people than me to look at this. > >> + device node | cs-gpio | CS pin state active | Note >> + +===+=+= >> + spi-cs-high | - | H | >> + - | - | L | >> + spi-cs-high | ACTIVE_HIGH | H | >> + - | ACTIVE_HIGH | L (or H ???)| 1 > > When using GPIO descriptors it will be enforced to ACTIVE_LOW (L) with an > explicit warning in dmesg, see drivers/gpio/gpiolib-of.c Ok, so in this line the L is ok. > > When using legacy GPIOs, will be enforced ACTIVE_LOW by the SPI > core. > >> + spi-cs-high | ACTIVE_LOW| H (or L ???)| 2 > > When using GPIO descriptors it will be enforced to ACTIVE_HIGH (H) with an > explicit warning in dmesg, see drivers/gpio/gpiolib-of.c Ok, so my assumption about H is right and not the part in parenthesis with ???. > >> + 3) Effectively this rule defines that the ACTIVE level of the >> + gpio has to be ignored > > Nr 3 isn't tagged in the table. Well, it was thought as a third but general note. Maybe should have been a *) or omitted since the table stands for itself. > > Yours, > Linus Walleij So let me prepare a patch with fixes for this. BR and thanks, Nikolaus
Re: [BUG] SPI broken for SPI based panel drivers
Hi Andreas, > Am 09.12.2020 um 09:04 schrieb Andreas Kemnade : > > Hi, > > On Sat, 5 Dec 2020 08:04:25 +0100 > "H. Nikolaus Schaller" wrote: > >> Hi Linus, >> >>> Am 05.12.2020 um 01:25 schrieb Linus Walleij : >>> >>> On Fri, Dec 4, 2020 at 5:52 PM H. Nikolaus Schaller >>> wrote: >>> >>>> But what I don't know is if I can omit spi-cs-high and have to keep >>>> ACTIVE_HIGH (my revert patch) or also change to ACTIVE_LOW (my additional >>>> patch). This is arbitrary and someone has to decide what it should be. >>> (...) >>>> I'd prefer if you or maybe Linus could submit such a patch and I am happy >>>> to review it. >>> >>> It seems really ill-advised to have me do that since I have not >>> managed very well to deal with this. Clearly better developers >>> are needed. But I can review a patch and see if it makes me >>> smarter :) > > Hmm, if those developers are not available, then probably finding > those bugs has to be time-optimized, like establishing better automatic > display testing. Well, I don't think we need automatic display testing. We just need to test if any SPI CS behaves correctly according to some specification. Then all displays and other chips will work - unless they have a bug in their DT. Basically it would need a software unit-test going through all 6 variants of having spi-cs-high and gpiod and parameters and looking if the chip (maybe some SPI EEPROM with known SPI polarity) responds or not. This can be done with hardware SPI controllers and spi-gpio. And it can be re-run each time something significant in gpiolib or spi-gpio is changed. > >> >> I find it interesting that so far nobody wants to take responsibility >> for a decision and to write down the behaviour really should be. Coding >> is the second step then. >> > well, the interesting people are not involved yet (DTML) because no > patch is sent. Well, I think we (gpiolib maintainers, spi maintainers and users of it) are the right ones to define the truth table. This is not primarily a DT issue, it is a matter of what we want to have and then it is cast it into DT (documentation). So I am not sure if delegation to someone else solves it. > >> Anyways you did not cite the really important part of my mail. So let me >> copy it back. Here it is again: >> >>> What I can do is to provide just a skeleton for the table that you or Linus >>> can fix/fill in and make a patch out of it. Is attached and the ??? is >>> something you should discuss and define. >> >> Please take the attached diff, comment it here and define the question marks >> according to your intention and then make a patch for the YAML bindings out >> of it. (I can't do because I don't know your intentions and what to write >> into >> the commit message). >> > Well, I the easiest step forward is just to document clearer how things > behave now, so the commit message is just something like > > "Behavior of CS signal is not clearly documented, clarify the > documentation". And then send the patch to the proper mailing lists > including devicetree folks. Ok, that looks like a good solution to get out of the deadlock. BR and thanks, Nikolaus > > Regards, > Andreas > >> As soon as we have settled this, we can check if code is correct and really >> define if my device tree fits and which change it needs exactly. >> >> BR and thanks, >> Nikolaus >> >> [slightly edited] >> >> diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml >> b/Documentation/devicetree/bindings/spi/spi-controller.yaml >> index 1b56d5e40f1f..4f8755dabecc 100644 >> --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml >> +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml >> @@ -42,6 +42,30 @@ properties: >>cs2 : 1 0 >>cs3 : 2 0 >> >> + The second flag of a gpio descriptor can be GPIO_ACTIVE_HIGH/0 >> + or GPIO_ACTIVE_LOW/1. >> + >> + There is a special rule set for combining the second flag of an >> + cs-gpio with the optional spi-cs-high flag for SPI slaves. >> + >> + Each table entry defines how the CS pin is physically driven >> + (not considering potential gpio inversions by pinmux): >> + >> + device node | cs-gpio | CS pin state active | Note >> + +===+=+= >> + spi-cs-high | - | H | >> + -
[PATCH] clk: ti: omap5: Fix reboot DPLL lock failure when using ABE TIMERs
From: David Shah
Having the ABE DPLL ref and bypass muxes set to different inputs was
causing the DPLL not to lock when TIMER8 was used, as it is in the Pyra
for the backlight.
This patch fixes this by setting abe_dpll_bypass_clk_mux to sys_32k_ck
in omap5xxx_dt_clk_init.
A similar patch may also be needed for OMAP44xx which has similar code
in omap4xxx_dt_clk_init, but I have not added this as I have no hardware
to test on.
Signed-off-by: David Shah
Signed-off-by: H. Nikolaus Schaller
---
drivers/clk/ti/clk-54xx.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/ti/clk-54xx.c b/drivers/clk/ti/clk-54xx.c
index 8694bc9f5fc7f..f0542391ca4bd 100644
--- a/drivers/clk/ti/clk-54xx.c
+++ b/drivers/clk/ti/clk-54xx.c
@@ -605,7 +605,7 @@ static struct ti_dt_clk omap54xx_clks[] = {
int __init omap5xxx_dt_clk_init(void)
{
int rc;
- struct clk *abe_dpll_ref, *abe_dpll, *sys_32k_ck, *usb_dpll;
+ struct clk *abe_dpll_ref, *abe_dpll, *abe_dpll_byp, *sys_32k_ck,
*usb_dpll;
ti_dt_clocks_register(omap54xx_clks);
@@ -616,6 +616,16 @@ int __init omap5xxx_dt_clk_init(void)
abe_dpll_ref = clk_get_sys(NULL, "abe_dpll_clk_mux");
sys_32k_ck = clk_get_sys(NULL, "sys_32k_ck");
rc = clk_set_parent(abe_dpll_ref, sys_32k_ck);
+
+ /*
+* This must also be set to sys_32k_ck to match or
+* the ABE DPLL will not lock on a warm reboot when
+* ABE timers are used.
+*/
+ abe_dpll_byp = clk_get_sys(NULL, "abe_dpll_bypass_clk_mux");
+ if (!rc)
+ rc = clk_set_parent(abe_dpll_byp, sys_32k_ck);
+
abe_dpll = clk_get_sys(NULL, "dpll_abe_ck");
if (!rc)
rc = clk_set_rate(abe_dpll, OMAP5_DPLL_ABE_DEFFREQ);
--
2.26.2
Re: [BUG] SPI broken for SPI based panel drivers
Hi Linus, > Am 05.12.2020 um 01:25 schrieb Linus Walleij : > > On Fri, Dec 4, 2020 at 5:52 PM H. Nikolaus Schaller > wrote: > >> But what I don't know is if I can omit spi-cs-high and have to keep >> ACTIVE_HIGH (my revert patch) or also change to ACTIVE_LOW (my additional >> patch). This is arbitrary and someone has to decide what it should be. > (...) >> I'd prefer if you or maybe Linus could submit such a patch and I am happy to >> review it. > > It seems really ill-advised to have me do that since I have not > managed very well to deal with this. Clearly better developers > are needed. But I can review a patch and see if it makes me > smarter :) I find it interesting that so far nobody wants to take responsibility for a decision and to write down the behaviour really should be. Coding is the second step then. Anyways you did not cite the really important part of my mail. So let me copy it back. Here it is again: > What I can do is to provide just a skeleton for the table that you or Linus > can fix/fill in and make a patch out of it. Is attached and the ??? is > something you should discuss and define. Please take the attached diff, comment it here and define the question marks according to your intention and then make a patch for the YAML bindings out of it. (I can't do because I don't know your intentions and what to write into the commit message). As soon as we have settled this, we can check if code is correct and really define if my device tree fits and which change it needs exactly. BR and thanks, Nikolaus [slightly edited] diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml index 1b56d5e40f1f..4f8755dabecc 100644 --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml @@ -42,6 +42,30 @@ properties: cs2 : 1 0 cs3 : 2 0 + The second flag of a gpio descriptor can be GPIO_ACTIVE_HIGH/0 + or GPIO_ACTIVE_LOW/1. + + There is a special rule set for combining the second flag of an + cs-gpio with the optional spi-cs-high flag for SPI slaves. + + Each table entry defines how the CS pin is physically driven + (not considering potential gpio inversions by pinmux): + + device node | cs-gpio | CS pin state active | Note + +===+=+= + spi-cs-high | - | H | + - | - | L | + spi-cs-high | ACTIVE_HIGH | H | + - | ACTIVE_HIGH | L (or H ???)| 1 + spi-cs-high | ACTIVE_LOW| H (or L ???)| 2 + - | ACTIVE_LOW| L | + + Notes: + 1) should print a warning about polarity inversion + because here it would be wise to define the gpio as ACTIVE_LOW + 2) could print a warning about polarity inversion + because ACTIVE_LOW is overridden by spi-cs-high + 3) Effectively this rule defines that the ACTIVE level of the + gpio has to be ignored + num-cs: $ref: /schemas/types.yaml#/definitions/uint32 description:
Re: [BUG] SPI broken for SPI based panel drivers
> Am 04.12.2020 um 20:19 schrieb Sven Van Asbroeck : > > On Fri, Dec 4, 2020 at 11:52 AM H. Nikolaus Schaller > wrote: >> >>>> Anyways it is debatable if this is a bug at all. It is just a definition. >>> >>> I respectfully disagree. Prior to the fix, your panel's active-low chip >>> select >>> needed to be described in the devicetree with 'spi-cs-high'. That sounds >>> very much like a bug to me. >> >> It could have been described by ACTIVE_LOW without spi-cs-high but that did >> emit a nasty and not helpful warning on each boot. > > That will not work, try it out. You will see that without the bugfix, your > chip > select is consistently inverted, no matter how you formulate it in the > devicetree. I have. But please show me which line in my analyses table of my mail 12 hours ago is wrong. Then I can repeat the test and we can discuss the reasons. > >> >> I'd prefer if you or maybe Linus could submit such a patch and I am happy to >> review it. > > I cannot help you with that, I'm sorry. Come on... BR and thanks, Nikolaus
Re: [BUG] SPI broken for SPI based panel drivers
Hi Sven,
> Am 04.12.2020 um 14:46 schrieb Sven Van Asbroeck :
>
> On Fri, Dec 4, 2020 at 5:11 AM H. Nikolaus Schaller
> wrote:
>>
>> Anyways it is debatable if this is a bug at all. It is just a definition.
>
> I respectfully disagree. Prior to the fix, your panel's active-low chip select
> needed to be described in the devicetree with 'spi-cs-high'. That sounds
> very much like a bug to me.
It could have been described by ACTIVE_LOW without spi-cs-high but that did
emit a nasty and not helpful warning on each boot.
Well, there are of course better and worse definitions and I agree that
spi-cs-high to define an active-low chip select sounds strange. Still it
is just a definition.
But what I don't know is if I can omit spi-cs-high and have to keep
ACTIVE_HIGH (my revert patch) or also change to ACTIVE_LOW (my additional
patch). This is arbitrary and someone has to decide what it should be.
Then, the gpiolib / spi driver code should be adapted if necessary and a
unit-test or real-hardware test with all possible combinations would prove
it for all other devices as well. So testing against a specification does
not need /my/ hardware.
>
>> Which is not well documented anywhere.
>
> I agree that documentation can be improved here.
> Would you like to submit a patch that improves:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/spi/spi-controller.yaml?h=v5.10-rc6#n28
> ?
Yes, that is the right location.
Basically it involves adding a table like in my previous mail to the comment
so that it also covers all cases where the second parameter is not 0.
I'd prefer if you or maybe Linus could submit such a patch and I am happy to
review it.
The reason is that I am neither involved in gpiolib nor spi subsystem
development
so I neither know what your intended setup is. I may define a wrong table.
I just need a stable definition by the subsystem maintainers so that I can
finish the device tree I am responsible for.
What I can do is to provide just a skeleton for the table that you or Linus
can fix/fill in and make a patch out of it. Is attached and the ??? is
something you should discuss and define.
> This way, we also get Rob Herring involved, which may lead to more elegant
> documentation. He is more likely to respond to a patch than to a question.
>
>>
>> What I especially wonder is why you fix that at all in drivers/spi/spi.c
>> if polarity inversion is handled in gpiolib.
>
> The reason for that is described in the commit message of
> 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors")
IMHO it could have been fixed in the gpiolib alone. In my assumption, gpiolib
would know (or be informed) that the gpio is used by spi and could invert
gpio_set_value()
if needed. Then any SPI code would simply use gpio_set_value(true) if
spi-cs-high
is defined and gpio_set_value(false) if not to enable the chip.
The alternative would be that it is only done centrally in one place in the
spi subsystem.
But I may be wrong, because the real architecture of the spi and gpiolib code
is quite new to me. My focus is on very different things (e.g. camera driver,
drm panel drivers) which is already complex enough.
BR and thanks,
Nikolaus
diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml
b/Documentation/devicetree/bindings/spi/spi-controller.yaml
index 1b56d5e40f1f..4f8755dabecc 100644
--- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
@@ -42,6 +42,30 @@ properties:
cs2 : 1 0
cs3 : 2 0
+ The second flag can be GPIO_ACTIVE_HIGH/0 or GPIO_ACTIVE_LOW/1.
+
+ There is special rule set for combining the second flag of an
+ cs-gpio with the optional spi-cs-high flag for SPI slaves.
+
+ Each table entry defines how the CS pin is physically driven
+ (not considering gpio inversions by pinmux):
+
+ device node | cs-gpio | CS pin state active | Note
+ +===+=+=
+ spi-cs-high | - | H |
+ - | - | L |
+ spi-cs-high | ACTIVE_HIGH | H |
+ - | ACTIVE_HIGH | L (or H ???)| 1
+ spi-cs-high | ACTIVE_LOW| H (or L ???)| 2
+ - | ACTIVE_LOW| L |
+
+ Notes:
+ 1) should print a warning about polarity inversion
+ because here it would be wise to define the gpio as ACTIVE_LOW
+ 2) could print a warning about polarity inversion
+ because ACTIVE_LOW is overridden by spi-cs-high
+ 3) Effectively this rule defines that the ACTIVE level of the
+ gpio has to be ignored
+
num-cs:
$ref: /schemas/types.yaml#/definitions/uint32
description:
Re: [BUG] SPI broken for SPI based panel drivers
Hi Sven and Linux,
> Am 01.12.2020 um 19:43 schrieb Sven Van Asbroeck :
>
> Hi Nikolaus,
I was about to submit two patches. One that reverts our "spi-cs-high"
and one that actively sets the GPIO_ACTIVE_LOW because that seems
to be the right thing. It would work with v5.10, but is it really
the right thing?
Before submitting I would like to clarify the rules.
Especially since it is the same discussion as before:
https://lkml.org/lkml/2019/3/23/131
https://lkml.org/lkml/2019/3/24/2
> On Tue, Dec 1, 2020 at 12:13 PM H. Nikolaus Schaller
> wrote:
>>> Am 01.12.2020 um 17:53 schrieb Sven Van Asbroeck :
>>> On Tue, Dec 1, 2020 at 11:43 AM H. Nikolaus Schaller
>>> wrote:
>>>>
>>>> You are right. It is active low.
>>>>
>>>
>>> In that case, we have a very simple solution, just remove the spi-cs-high,
>>> and things will work.
>>
>> We originally had it that way and because there was a change in gpiolib we
>> had
>> to introduce it.
>
> The current rules re. spi chip-selects in devicetrees are here:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib-of.c?h=v5.10-rc6#n191
Yes, there is
/*
* SPI children have active low chip selects
* by default. This can be specified negatively
* by just omitting "spi-cs-high" in the
* device node, or actively by tagging on
* GPIO_ACTIVE_LOW as flag in the device
* tree. If the line is simultaneously
* tagged as active low in the device tree
* and has the "spi-cs-high" set, we get a
* conflict and the "spi-cs-high" flag will
* take precedence.
*/
It is not complete and a formulated a little in reverse.
A table would would be more precise.
And it is not the complete rule:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib-of.c?h=v5.10-rc6#n175
/*
* Legacy handling of SPI active high chip select. If we have a
* property named "cs-gpios" we need to inspect the child node
* to determine if the flags should have inverted semantics.
*/
This seems to indicate that the gpio flags can invert spi-cs-high. There are
only two options: invert if they are GPIO_ACTIVE_LOW or GPIO_ACTIVE_HIGH.
This makes IMHO much sense. But it seems that it was not implemented that way.
>
> This is the way I see things:
> - according to the current rules, your devicetree describes a spi panel with
> an active-high chip select
I have now tested all 4 combinations of GPIO_ACTIVE_LOW/HIGH and missing/present
"spi-cs-high" with v5.4.79.
This leads to the following definition:
a) general SPI controller or if no cs-gpios available in spi-gpio
device node | CS pin state active
+==
spi-cs-high | H
- | L
This was the single (legacy) rule until v4.18 effectively ignoring any gpio
flags.
b) with v4.19 and 6953c57ab172 ("gpio: of: Handle SPI chipselect legacy
bindings") was introduced
it was apparently extended to (by code inspection of the patch):
device node | cs-gpio | CS pin state active
+===+
spi-cs-high | - | H
- | - | L
spi-cs-high | ACTIVE_HIGH | L (polarity inversion, no warning)
- | ACTIVE_HIGH | L + "enforce active low on chipselect handle"
spi-cs-high | ACTIVE_LOW| H + "GPIO handle specifies active low -
ignored"
- | ACTIVE_LOW| H (no warning)
But I did test all 4 combinations with v5.4.79 and assuming CS pin
state L if the panel works gave me:
device node | cs-gpio | CS pin state active
+===+
spi-cs-high | ACTIVE_HIGH | L (panel works)
- | ACTIVE_HIGH | H (panel fails) + "enforce active low on
chipselect handle"
spi-cs-high | ACTIVE_LOW| L (panel works) + "GPIO handle specifies
active low - ignored"
- | ACTIVE_LOW| H (panel fails without comment)
This means there was some additional tweak to the rules effectively ignoring
the cs-gpio. Like before - but with opposite polarity.
Note that some legacy gpio flags are 0 (ACTIVE_HIGH) so it seems to make sense
for the ACTIVE_HIGH case like ours.
Therefore, only GPIO_ACTIVE_HIGH + "spi-cs-high" works without side-effects
which
is the reason why we
Re: [PATCH] ARM: omap2plus_defconfig: enable SPI GPIO
> Am 01.12.2020 um 20:12 schrieb Andreas Kemnade : > > GTA04 uses that for controlling the td028ttec1 panel. So > for easier testing/bisecting it is useful to have it > enabled in the defconfig. ++ > > Signed-off-by: Andreas Kemnade > --- > arch/arm/configs/omap2plus_defconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/configs/omap2plus_defconfig > b/arch/arm/configs/omap2plus_defconfig > index 77716f500813..904a8757ad9f 100644 > --- a/arch/arm/configs/omap2plus_defconfig > +++ b/arch/arm/configs/omap2plus_defconfig > @@ -280,6 +280,7 @@ CONFIG_SERIAL_OMAP_CONSOLE=y > CONFIG_SERIAL_DEV_BUS=y > CONFIG_I2C_CHARDEV=y > CONFIG_SPI=y > +CONFIG_SPI_GPIO=m > CONFIG_SPI_OMAP24XX=y > CONFIG_SPI_TI_QSPI=m > CONFIG_HSI=m > -- > 2.20.1 >
Re: [BUG] SPI broken for SPI based panel drivers
Hi Sven,
> Am 01.12.2020 um 17:53 schrieb Sven Van Asbroeck :
>
> Hi Nikolaus,
>
> On Tue, Dec 1, 2020 at 11:43 AM H. Nikolaus Schaller
> wrote:
>>
>> You are right. It is active low.
>>
>
> In that case, we have a very simple solution, just remove the spi-cs-high,
> and things will work.
We originally had it that way and because there was a change in gpiolib we had
to introduce it.
See: f1f028ff89cb0d3
where we were forced to introduce it although I had preferred to not change DT.
I am not sure if DT maintainers accept that we revert a DT change just to
handle some change in a driver. Usually they insist on fixing a driver and
live with the DT. DT is carved in stone or could be ROM...
So you could try to submit a revert of f1f028ff89cb0d3 with a description
why it is needed. And please make sure that it is also applied where your
patch is backported to stable. So it should have some
Fixes: 766c6b63aa04 ("spi: fix client driver breakages when using GPIO
descriptors")
>
> In case of SPI CS gpios, the current kernel ignores all
> GPIO_ACTIVE_HIGH/LOW
> flags, and uses the presence/absence of spi-cs-high instead, to
> determine active high / active low.
So you mean you are just restoring the behaviour before
6953c57ab172
was introduced?
The alternative is that you restore the cs-gpios + spi-cs-high semantics
as defined by 6953c57ab172 and everything is fine without touching (our)
DTS (again).
BR and thanks,
Nikolaus
Re: [Letux-kernel] [BUG] SPI broken for SPI based panel drivers
> Am 01.12.2020 um 17:44 schrieb Andreas Kemnade : > > On Tue, 1 Dec 2020 11:10:49 -0500 > Sven Van Asbroeck wrote: > >> Nikolaus, >> >> On Tue, Dec 1, 2020 at 9:38 AM H. Nikolaus Schaller >> wrote: >>> >>> Let's work on a fix for the fix now. >>> >> >> Are you quite sure the chip-select of the tpo,td028ttec1 panel >> is active-high? A quick google produced a datasheet which >> seems to indicate that XCS is active-low? >> > Schematics is here: > https://projects.goldelico.com/p/gta04-main/downloads/48/ > > The display connector P204-LCD indicates some inversion at the XCS and > XRES pins. > > This patch fixes things for a boot where the display was not > touched by the bootloader > diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi > b/arch/arm/boot/dts/omap3-gta04.dtsi > index c8745bc800f7..003202d12990 100644 > --- a/arch/arm/boot/dts/omap3-gta04.dtsi > +++ b/arch/arm/boot/dts/omap3-gta04.dtsi > @@ -124,7 +124,6 @@ >spi-max-frequency = <10>; >spi-cpol; >spi-cpha; > - spi-cs-high; BTW: that is what I had planned to try next... > >backlight= <>; >label = "lcd"; > > So if that one is really active-low, why did it ever work?! > > Regards, > Andreas
Re: [Letux-kernel] [BUG] SPI broken for SPI based panel drivers
> Am 01.12.2020 um 17:44 schrieb Andreas Kemnade : > > On Tue, 1 Dec 2020 11:10:49 -0500 > Sven Van Asbroeck wrote: > >> Nikolaus, >> >> On Tue, Dec 1, 2020 at 9:38 AM H. Nikolaus Schaller >> wrote: >>> >>> Let's work on a fix for the fix now. >>> >> >> Are you quite sure the chip-select of the tpo,td028ttec1 panel >> is active-high? A quick google produced a datasheet which >> seems to indicate that XCS is active-low? >> > Schematics is here: > https://projects.goldelico.com/p/gta04-main/downloads/48/ > > The display connector P204-LCD indicates some inversion at the XCS and > XRES pins. > > This patch fixes things for a boot where the display was not > touched by the bootloader > diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi > b/arch/arm/boot/dts/omap3-gta04.dtsi > index c8745bc800f7..003202d12990 100644 > --- a/arch/arm/boot/dts/omap3-gta04.dtsi > +++ b/arch/arm/boot/dts/omap3-gta04.dtsi > @@ -124,7 +124,6 @@ >spi-max-frequency = <10>; >spi-cpol; >spi-cpha; > - spi-cs-high; > >backlight= <>; >label = "lcd"; > > So if that one is really active-low, why did it ever work?! Apparently, because there was patch f1f028ff89cb0d3 to fix 6953c57ab172 ... BR, Nikolaus
Re: [BUG] SPI broken for SPI based panel drivers
> Am 01.12.2020 um 16:52 schrieb Sven Van Asbroeck :
>
> Nikolaus,
>
> On Tue, Dec 1, 2020 at 9:38 AM H. Nikolaus Schaller
> wrote:
>>
>> Let's work on a fix for the fix now.
>
> I tested spi-gpio on my system, by converting a built-in or hardware spi,
> to a spi-gpio. Interestingly, the patch has the opposite effect on my system:
> before the patch, spi-gpio did not work, but after it's applied, it does work.
>
> Can you tell me the idle status of your chip-select gpio in debugfs?
> # mount -t debugfs none /sys/kernel/debug
> # cat /sys/kernel/debug/gpio
> Look for something like this:
> gpiochip0: GPIOs 0-31, parent: platform/209c000.gpio, 209c000.gpio:
> gpio-17 (|spi5 CS0) out hi ACTIVE LOW
root@letux:~# mount -t debugfs none /sys/kernel/debug
root@letux:~# cat /sys/kernel/debug/gpio|fgrep spi
gpio-179 (|spi4 CS0) out lo
root@letux:~#
That is after the panel driver did send the commands.
>
> Also, apply the following patch, and tell me
> a) does this dev_err() get called on your system, and
yes. Many times.
> b) what is the value when your chip is de-selected
root@letux:~# dmesg|fgrep td028
[ 14.530456] panel-tpo-td028ttec1 spi4.0: spi->mode = 0003
[ 14.599212] panel-tpo-td028ttec1 spi4.0: gpiod disable
[ 14.817871] panel-tpo-td028ttec1 spi4.0: spi->mode = 0003
[ 14.817871] panel-tpo-td028ttec1 spi4.0: gpiod enable
So it is disabled first and then enabled. Which appears to be the opposite
of what it should be.
BTW: I have added another dev_err to print the spi->mode and like
you describe it is (overwritten? by SPI_MODE_3.
I can check what value it has in the driver before it is set to SPI_MODE_3.
Maybe, there the spi-cs-high gets lost?
BR and thanks,
Nikolaus
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 7e8804b02be9..b2f4cf5c9ffb 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -813,11 +813,12 @@ static void spi_set_cs(struct spi_device *spi,
> bool enable)
>
>if (spi->cs_gpiod || gpio_is_valid(spi->cs_gpio)) {
>if (!(spi->mode & SPI_NO_CS)) {
> - if (spi->cs_gpiod)
> + if (spi->cs_gpiod) {
> + dev_err(>dev, "gpiod %s", enable1
> ? "enable" : "disable");
>/* polarity handled by gpiolib */
>gpiod_set_value_cansleep(spi->cs_gpiod,
> enable1);
> - else
> + } else
>/*
> * invert the enable line, as active low is
> * default for SPI.
Re: [BUG] SPI broken for SPI based panel drivers
> Am 01.12.2020 um 17:20 schrieb Mark Brown : > > On Tue, Dec 01, 2020 at 03:20:12PM +0100, Linus Walleij wrote: > >> The reason why I shoot in the dark to convert all SPI >> drivers to use GPIO descriptors instead of the global >> GPIO numberspace is detailed in drivers/gpio/TODO >> so I will not repeat it here. > >> I don't know if much can be done about it other than >> having better programmers than me at the task. Or >> less tired when they write the patch. etc. > > I think the problem here is more to do with where we started than where > we're going or how we got there - things have been glued together or > happened to work in ways that mean I'm not sure we reasonably understand > the situation we started from or all the requirements it has. As you > say I'm not sure anything beyond throwing the API away and starting > afresh would really help here, but that's not really how we tend to do > things for a bunch of very good reasons. I think the key problem is GPIO_ACTIVE_HIGH 0 and GPIO_ACTIVE_LOW 1 in device tree blobs. But that is so fundamental that we have to live with it. So I guess that even a new API from scratch wouldn't improve that.
Re: [BUG] SPI broken for SPI based panel drivers
> Am 01.12.2020 um 17:10 schrieb Sven Van Asbroeck : > > Nikolaus, > > On Tue, Dec 1, 2020 at 9:38 AM H. Nikolaus Schaller > wrote: >> >> Let's work on a fix for the fix now. >> > > Are you quite sure the chip-select of the tpo,td028ttec1 panel > is active-high? A quick google produced a datasheet which > seems to indicate that XCS is active-low? > > See page 17 here: > http://www.lcd-source.com/datasheet/TPO/TD028TTEC1.pdf > > It is of course possible that you are driving that line behind > some inverting circuitry. Hardware designers seem to do that > all the time, if they need to go from one voltage domain to > the other, etc. You are right. It is active low. But that is the start of a long story... It was introduced in DT by c2e138bc8ed80ae with cs-gpios = < 19 0>; because the third parameter did not have any meaning back then, AFAIR. It was ignored and drivers did initialize gpios with inversion if needed. Missing spi-cs-high; did define active low back then for spi-gpio. In 3a637e008e542b8ebdc2ceed616b229af0462b14 the "0" was replaced by the constant cs-gpios = < 19 GPIO_ACTIVE_HIGH>; keeping the DTB unchanged and compatible. This did work because there was still no spi-cs-high; Then came 6953c57ab172 which mixed cs-gpios and spi-cs-high; and made use of the now wrong GPIO_ACTIVE_HIGH; A special logic converted GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW in case of spi-cs-high; being present. A long discussion revealed that the only solution to stay compatible with old and new DT/kernels was to add spi-cs-high; and keep the wrong < 19 GPIO_ACTIVE_HIGH>; f1f028ff89cb0d3 did "fix" this by adding spi-cs-high to the DT. Please note that apparently we were already confused about what the right value should be (ACTIVE_HIGH or ACTIVE_LOW) in 2019 but the result was working until your new patch appeared. Yes, it could be that the problem introduced by 6953c57ab172 is just coming up again with your new change. On the other hand I remember from the old discussion that changing a DT is "forbidden" if it is not upwards compatible with earlier kernels. The driver must take what it gets from (legacy) DT. BR, Nikolaus
Re: [BUG] SPI broken for SPI based panel drivers
Hi Linus, > Am 01.12.2020 um 15:20 schrieb Linus Walleij : > > Hi Nikolaus, Sven, > > the fault that the thing broke in the first place is all mine. > Credit where credit is due! > > The reason why I shoot in the dark to convert all SPI > drivers to use GPIO descriptors instead of the global > GPIO numberspace is detailed in drivers/gpio/TODO > so I will not repeat it here. > > I don't know if much can be done about it other than > having better programmers than me at the task. Or > less tired when they write the patch. etc. > > What other operating systems do to get around the same > type of refactoring problem is to aggressively > deprecate and delete code that does not follow the > latest ideas of the driver subsystem developer. This > is not an option on Linux because we don't like to leave > working hardware and users behind so I am painstakingly > fixing it all over the place, with a little help from my > friends. Sometimes it blows up in my face, sometimes > in other people faces too, sorry about that. Everyone has such a patch written at the wrong time of the day... See for example: f069b5a0b27a Let's work on a fix for the fix now. BR and thanks, Nikolaus
Re: [BUG] SPI broken for SPI based panel drivers
> Am 01.12.2020 um 13:16 schrieb Mark Brown : > > On Tue, Dec 01, 2020 at 09:59:43AM +0100, H. Nikolaus Schaller wrote: >>> Am 30.11.2020 um 21:13 schrieb Sven Van Asbroeck : > >>> We knew that there was a chance that our fix would break something else. >>> But hopefully "it fixes more than it breaks" > >> Then it should not have been applied to mainline but fully worked out >> and tested. > > If you want to see better testing of things in mainline please > contribute to the various kernel testing efforts that are out there, > there's a huge range of systems out there using the kernel and it's > simply not realistic to expect that they'll all be covered, the testing > effort for the kernel is very much a community effort. If there are > things that are particularly important to you the best way to ensure > that they are tested is to provide that testing yourself. Well, having a working display of a portable device that has mainline Linux support for many years is particularly important... BTW, I am already part of this testing efforts out there. Because I test almost all -rc when they arrive. So I just did what you propose, And if I can locate the commit of a regression I write bug reports like this one. The only thing I could have done differently is to always test based on linux-next but that is a less structured testing environment and has its own pitfalls. But that would have revealed the issue only earlier but not with less effort or with a quicker fix. I am not sure if ít is realistic to dream of some way of informing/contacting the potentially affected driver authors/maintainers to run a quick test before it is merged upstream. To be clear: I do not expect that there are no bugs at all (for that I know Linux and software far too long). But I do not expect regression of the type hopefully "it fixes more than it breaks". Well, for me it (apparently) breaks more than it fixes. So the easiest fix for me would be to revert the patch. But I am sure that then you are not happy with it... So let's set out for a better solution. BR and thanks, Nikolaus
Re: [BUG] SPI broken for SPI based panel drivers
Hi Sven, > Am 01.12.2020 um 13:41 schrieb Sven Van Asbroeck : > > On Tue, Dec 1, 2020 at 4:04 AM H. Nikolaus Schaller > wrote: >> >> Then it should not have been applied to mainline but fully worked out and >> tested. Well I only complain because you wrote that you knew that it may break something else. So it is known to induces a regression. But I did not know until I got a report from our own testing effort (we run a vendor kernel and do tests all days over the week). So I had to bisect what was going on. There could have been 100 reasons. Took me several hours because I had to look at the display to see if it is on or off after boot. There was no simple shell command to find out and let the bisect run over night. Maybe printing a "please check your spi setup" in spi_setup() with a comment hinting at your patch would have saved me a lot of time. > > That would be a reasonable expectation of a product. But Linux > isn't a product, it's a hugely complex, shared system, which may > form the basis of your product. The core maintainers aren't > superhuman, nor do they have access to the 1000s of configurations > and devices where Linux runs or will run. They do their very best, > but if every change had to be 100% tested in every possible > configuration, then few things could ever change, and Linux > would slow down to a snail's pace. > When your product is based on Linux and you pull a newer version > off kernel.org, it's not unreasonable to expect the occasional > breakage. In my case, when I moved from 5.7 to 5.9, some of the > things that broke were my network chip, and most SPI drivers. That > was a bad day, most pulls are trouble-free. If it were occasional it would be good It is not the first time that I have to do bisect tests to find what did go wrong upstream. Even in LTS kernels and some of my fixes were backported later. > I believe LTSes are more stable than 'stable releases' which are in > turn more stable than RCs. The choice involves a trade-off between > features, security and stability. > > When you do run into a breakage, complaining on the mailing list > is good, but posting a fix is better :) I usually do - if I have the time to go the extra mile to fix something what somebody else did break. But I need to have a lot of spare time if I have no idea what made the regression for a device driver where I know that it was not touched and have to study the details. And for me it is much less effort (hours vs. seconds) to do a revert... I could submit that as a quick fix :) But then you are not happy. > This is my layman's understanding of the situation, I'm just a user > and not a maintainer. Me too... Well, I am sort of maintainer of a vendor kernel that tries to follow linus/master and fix things before we release an LTS. That is why I need a solution and can not go back to some LTS or wait for the next one... Anyways, there is still time until v5.10.0 to fix it better than by a revert. > >>> >>>> >>>> What should we do? > > Hopefully I have some time this week to look into your breakage, > I may get overtaken by someone much more knowledgeable than > me on spi-gpio. Hm. Then we are already two of us who have not much knowledge about spi-gpio... Hope that you have an idea soon. I am happy to test any suggestions/patches/alternatives better than a simple revert. BR and thanks, Nikolaus
Re: [BUG] SPI broken for SPI based panel drivers
Hi Sven,
> Am 30.11.2020 um 21:13 schrieb Sven Van Asbroeck :
>
> Hi Nikolaus, thank you for reaching out !
>
> On Mon, Nov 30, 2020 at 2:06 PM H. Nikolaus Schaller
> wrote:
>>
>> But reverting your patch brings back the display. So it appears as if it
>> does not
>> fix a breakage, rather breaks a previously working setup.
>
> The patch in question fixes an important breakage: before the patch, literally
> hundreds of SPI drivers no longer worked - only if the SPI bus master
> driver was using gpio descriptors.
>
> We knew that there was a chance that our fix would break something else.
> But hopefully "it fixes more than it breaks"
Then it should not have been applied to mainline but fully worked out and
tested.
>
>>
>> What should we do?
>>
>
> Can you try the following patch ?
Unfortunately it doesn't seem to fix it. And combined with the second proposed
fix also not.
BR and thanks,
Nikolaus
my combined change:
diff --git a/drivers/spi/spi-gpio.c b/drivers/spi/spi-gpio.c
index 7ceb0ba27b755c..ec2da62716a279 100644
--- a/drivers/spi/spi-gpio.c
+++ b/drivers/spi/spi-gpio.c
@@ -208,8 +208,8 @@ static void spi_gpio_chipselect(struct spi_device *spi, int
is_active)
if (spi_gpio->cs_gpios) {
struct gpio_desc *cs = spi_gpio->cs_gpios[spi->chip_select];
- /* SPI chip selects are normally active-low */
- gpiod_set_value_cansleep(cs, (spi->mode & SPI_CS_HIGH) ?
is_active : !is_active);
+ /* SPI chip select polarity handled by gpiolib*/
+ gpiod_set_value_cansleep(cs, is_active);
}
}
@@ -226,8 +226,7 @@ static int spi_gpio_setup(struct spi_device *spi)
if (spi_gpio->cs_gpios) {
cs = spi_gpio->cs_gpios[spi->chip_select];
if (!spi->controller_state && cs)
- status = gpiod_direction_output(cs,
- !(spi->mode & SPI_CS_HIGH));
+ status = gpiod_direction_output(cs, false);
}
if (!status)
[BUG] SPI broken for SPI based panel drivers
Hi,
starting from v5.10-rc5 the SPI based panel of the GTA04 device is broken. It
uses
compatible = "spi-gpio"; [1] i.e. gpio descriptors very indirectly.
Bisect shows that it is commit
766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors")
The commit description tells about a problematic pattern and indeed the driver
is
using it - like ca. 15 other spi based panel drivers in drivers/gpu/drm/panel/
I understood that it wants to fix the spi system to handle that correctly again.
But reverting your patch brings back the display. So it appears as if it does
not
fix a breakage, rather breaks a previously working setup.
What should we do?
BR and thanks,
Nikolaus
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/omap3-gta04.dtsi?h=v5.10-rc6#n107
Re: [PATCH 1/2] MIPS: Ingenic: Add missing nodes for Ingenic SoCs and boards. [and discussion about jz4780 SMP/MMC/Ethernet]
> Am 08.11.2020 um 16:13 schrieb Zhou Yanjie : > > > On 2020/11/8 下午10:35, H. Nikolaus Schaller wrote: >>> Am 08.11.2020 um 13:46 schrieb Zhou Yanjie : >>> >>> Hello Nikolaus, >>> >>> On 2020/11/8 上午3:03, H. Nikolaus Schaller wrote: >>>>> Am 07.11.2020 um 12:52 schrieb 周琰杰 (Zhou Yanjie) >>>>> : >>>>> >>>>> 1.Add OTG/OTG PHY/RNG nodes for JZ4780, CGU/OTG nodes for CI20. >>>>> 2.Add OTG/OTG PHY/RNG/OST nodes for X1000, SSI/CGU/OST/OTG/SC16IS752 >>>>> nodes for CU1000-Neo. >>>>> 3.Add OTG/OTG PHY/DTRNG/OST nodes for X1830, SSI/CGU/OST/OTG/SC16IS752 >>>>> nodes for CU1830-Neo. >>>>> >>>>> Tested-by: 周正 (Zhou Zheng) >>>> Comments below for CI20 / jz4780 only. >>>> >>>> I tried to test on top of my v5.10 working tree but the CI20 does not boot >>>> any more. >>>> Maybe the SMP hacks are in the way and I have to remove them first or try >>>> on top >>>> of v5.9.y >>> >>> SMP is not available on the mainline now, so the previous SMP patch needs >>> to be removed. >> I have tried with CONFIG_SMP disabled and now I can boot my v5.10-rc2 (plus >> local patches). >> And can confirm that the OTG port works. > > > Nice to hear it. > > >> (BTW: dm9000 defers probing forever but that is an unrelated issue). > > > On my side , it will work normally after selecting CONFIG_JZ4780_EFUSE. Seems to be related to some vcc-supply: [ 14.383309] dm9000 1600.dm9000: Looking up vcc-supply from device tree [ 14.442598] [ cut here ] [ 14.442626] WARNING: CPU: 0 PID: 19 at drivers/regulator/core.c:2125 _regulator_put+0x4c/0x104 [ 14.442631] Modules linked in: ohci_platform ingenic_drm(+) gpio_keys gpio_ir_recv ohci_hcd ehci_platform dw_hdmi dwc2 roles ehci_hcd drm_kms_helper dm9000 mii rtc_pcf8563 syscopyarea sysfillrect sysimgblt fb_sys_fops drm drm_panel_orientation_quirks ipv6 autofs4 [ 14.442723] CPU: 0 PID: 19 Comm: kworker/0:1 Tainted: GW 5.10.0-rc2-letux-mips+ #3790 [ 14.442738] Workqueue: events deferred_probe_work_func [ 14.442745] Stack : 83941bd4 80165d58 80b7 80b7 80aaf3dc 80adad20 0009 [ 14.442775] 084d 80165da4 80b6ff37 83936978 80b7 0001 83941b98 f6d2dfd3 [ 14.442805] 80aaf3dc 000c 0114 0002 0001 746e6576 [ 14.442833] 65642073 5c11 000f 72726566 80b7 80590ad0 80adad20 [ 14.442862] 0009 084d 0001 80cb 805f6a28 0012829f 001282df [ 14.442891] ... [ 14.442901] Call Trace: [ 14.442918] [<8010912c>] show_stack+0x6c/0x12c [ 14.442933] [<801229e4>] __warn+0xd4/0x108 [ 14.442943] [<80122aa0>] warn_slowpath_fmt+0x88/0xc8 [ 14.442953] [<80590ad0>] _regulator_put+0x4c/0x104 [ 14.442961] [<80590bb0>] regulator_put+0x28/0x40 [ 14.442998] [<805dac8c>] release_nodes+0x220/0x280 [ 14.443013] [<808b1488>] really_probe+0x344/0x464 [ 14.443022] [<805d612c>] driver_probe_device+0x1d8/0x224 [ 14.443038] [<805d3efc>] bus_for_each_drv+0x90/0xc8 [ 14.443047] [<805d5e5c>] __device_attach+0xc8/0x174 [ 14.443057] [<805d4e3c>] bus_probe_device+0x3c/0xb4 [ 14.443066] [<805d53d4>] deferred_probe_work_func+0x7c/0xb4 [ 14.443078] [<8013d574>] process_one_work+0x204/0x348 [ 14.443087] [<8013e028>] worker_thread+0x28c/0x3c0 [ 14.443101] [<80143ac8>] kthread+0x14c/0x154 [ 14.443111] [<80102c0c>] ret_from_kernel_thread+0x14/0x1c [ 14.443119] [ 14.443126] ---[ end trace 76225a8e653eb480 ]--- [ 14.443379] platform 1600.dm9000: Driver dm9000 requests probe deferral > > >>> The new SMP patch will take some time because it needs to ensure support >>> for the new X2000 processor. >> Ok, I see. It is a little sad since it works find until 5.9 but now is >> broken and means we >> have to test v5.10ff with only one processor. Any hints what should be >> hacked to make it >> work until a final solution becomes available in upstream? > > > I can try to make a version that supports JZ4780 first, and I will send it to > you when it is completed. Would be nice! > > >>> >>> Thanks and best regards! >> If you like you can add my >> >> Tested by: H. Nikolaus Schaller # CI20/jz4780 >> >> to this patch. > > > Sure, I will add it to the next version. > > > BTW, have you noticed the problem with the MMC of JZ4780? This was first > discovered by Paul. After he tol
Re: [PATCH 1/2] MIPS: Ingenic: Add missing nodes for Ingenic SoCs and boards.
> Am 08.11.2020 um 13:46 schrieb Zhou Yanjie :
>
> Hello Nikolaus,
>
> On 2020/11/8 上午3:03, H. Nikolaus Schaller wrote:
>>> Am 07.11.2020 um 12:52 schrieb 周琰杰 (Zhou Yanjie)
>>> :
>>>
>>> 1.Add OTG/OTG PHY/RNG nodes for JZ4780, CGU/OTG nodes for CI20.
>>> 2.Add OTG/OTG PHY/RNG/OST nodes for X1000, SSI/CGU/OST/OTG/SC16IS752
>>> nodes for CU1000-Neo.
>>> 3.Add OTG/OTG PHY/DTRNG/OST nodes for X1830, SSI/CGU/OST/OTG/SC16IS752
>>> nodes for CU1830-Neo.
>>>
>>> Tested-by: 周正 (Zhou Zheng)
>> Comments below for CI20 / jz4780 only.
>>
>> I tried to test on top of my v5.10 working tree but the CI20 does not boot
>> any more.
>> Maybe the SMP hacks are in the way and I have to remove them first or try on
>> top
>> of v5.9.y
>
>
> SMP is not available on the mainline now, so the previous SMP patch needs to
> be removed.
I have tried with CONFIG_SMP disabled and now I can boot my v5.10-rc2 (plus
local patches).
And can confirm that the OTG port works.
(BTW: dm9000 defers probing forever but that is an unrelated issue).
> The new SMP patch will take some time because it needs to ensure support for
> the new X2000 processor.
Ok, I see. It is a little sad since it works find until 5.9 but now is broken
and means we
have to test v5.10ff with only one processor. Any hints what should be hacked
to make it
work until a final solution becomes available in upstream?
>
>
> Thanks and best regards!
If you like you can add my
Tested by: H. Nikolaus Schaller # CI20/jz4780
to this patch.
>
>
>>
>>> Signed-off-by: 周琰杰 (Zhou Yanjie)
>>> ---
>>> arch/mips/boot/dts/ingenic/ci20.dts | 16 +
>>> arch/mips/boot/dts/ingenic/cu1000-neo.dts | 60
>>> +++
>>> arch/mips/boot/dts/ingenic/cu1830-neo.dts | 60
>>> +++
>>> arch/mips/boot/dts/ingenic/jz4780.dtsi| 41 +++--
>>> arch/mips/boot/dts/ingenic/x1000.dtsi | 52 ++-
>>> arch/mips/boot/dts/ingenic/x1830.dtsi | 54 +++-
>>> 6 files changed, 267 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts
>>> b/arch/mips/boot/dts/ingenic/ci20.dts
>>> index 75f5bfbf2c37..b31054a41754 100644
>>> --- a/arch/mips/boot/dts/ingenic/ci20.dts
>>> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
>>> @@ -93,6 +93,15 @@
>>> clock-frequency = <4800>;
>>> };
>>>
>>> + {
>>> + /*
>>> +* Use the 32.768 kHz oscillator as the parent of the RTC for a higher
>>> +* precision.
>>> +*/
>>> + assigned-clocks = < JZ4780_CLK_RTC>;
>>> + assigned-clock-parents = < JZ4780_CLK_RTCLK>;
>>> +};
>>> +
>> ok
>>
>>> {
>>> status = "okay";
>>>
>>> @@ -396,6 +405,13 @@
>>> status = "okay";
>>> };
>>>
>>> + {
>>> + status = "okay";
>>> +
>>> + assigned-clocks = < JZ4780_CLK_OTGPHY>;
>>> + assigned-clock-rates = <4800>;
>>> +};
>>> +
>> ok
>>
>>> {
>>> pins_uart0: uart0 {
>>> function = "uart0";
>>> diff --git a/arch/mips/boot/dts/ingenic/cu1000-neo.dts
>>> b/arch/mips/boot/dts/ingenic/cu1000-neo.dts
>>> index 22a1066d637b..44d47d12db12 100644
>>> --- a/arch/mips/boot/dts/ingenic/cu1000-neo.dts
>>> +++ b/arch/mips/boot/dts/ingenic/cu1000-neo.dts
>>> @@ -3,7 +3,7 @@
>>>
>>> #include "x1000.dtsi"
>>> #include
>>> -#include
>>> +#include
>>> #include
>>>
>>> / {
>>> @@ -31,6 +31,18 @@
>>> };
>>> };
>>>
>>> + ssi: spi-gpio {
>>> + compatible = "spi-gpio";
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + num-chipselects = <1>;
>>> +
>>> + mosi-gpios = < 2 GPIO_ACTIVE_HIGH>;
>>> + miso-gpios = < 3 GPIO_ACTIVE_HIGH>;
>>> + sck-gpios = < 0 GPIO_ACTIVE_HIGH>;
>>> + cs-gpios = < 1 GPIO_ACTIVE_HIGH>;
>>> + };
>>> +
>>> wlan_pwrseq: msc1-pwrseq {
>>> compatible = "mmc-pwrseq-simple
Re: [PATCH 1/2] MIPS: Ingenic: Add missing nodes for Ingenic SoCs and boards.
> Am 07.11.2020 um 12:52 schrieb 周琰杰 (Zhou Yanjie) :
>
> 1.Add OTG/OTG PHY/RNG nodes for JZ4780, CGU/OTG nodes for CI20.
> 2.Add OTG/OTG PHY/RNG/OST nodes for X1000, SSI/CGU/OST/OTG/SC16IS752
> nodes for CU1000-Neo.
> 3.Add OTG/OTG PHY/DTRNG/OST nodes for X1830, SSI/CGU/OST/OTG/SC16IS752
> nodes for CU1830-Neo.
>
> Tested-by: 周正 (Zhou Zheng)
Comments below for CI20 / jz4780 only.
I tried to test on top of my v5.10 working tree but the CI20 does not boot any
more.
Maybe the SMP hacks are in the way and I have to remove them first or try on top
of v5.9.y
> Signed-off-by: 周琰杰 (Zhou Yanjie)
> ---
> arch/mips/boot/dts/ingenic/ci20.dts | 16 +
> arch/mips/boot/dts/ingenic/cu1000-neo.dts | 60 +++
> arch/mips/boot/dts/ingenic/cu1830-neo.dts | 60 +++
> arch/mips/boot/dts/ingenic/jz4780.dtsi| 41 +++--
> arch/mips/boot/dts/ingenic/x1000.dtsi | 52 ++-
> arch/mips/boot/dts/ingenic/x1830.dtsi | 54 +++-
> 6 files changed, 267 insertions(+), 16 deletions(-)
>
> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts
> b/arch/mips/boot/dts/ingenic/ci20.dts
> index 75f5bfbf2c37..b31054a41754 100644
> --- a/arch/mips/boot/dts/ingenic/ci20.dts
> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
> @@ -93,6 +93,15 @@
> clock-frequency = <4800>;
> };
>
> + {
> + /*
> + * Use the 32.768 kHz oscillator as the parent of the RTC for a higher
> + * precision.
> + */
> + assigned-clocks = < JZ4780_CLK_RTC>;
> + assigned-clock-parents = < JZ4780_CLK_RTCLK>;
> +};
> +
ok
> {
> status = "okay";
>
> @@ -396,6 +405,13 @@
> status = "okay";
> };
>
> + {
> + status = "okay";
> +
> + assigned-clocks = < JZ4780_CLK_OTGPHY>;
> + assigned-clock-rates = <4800>;
> +};
> +
ok
> {
> pins_uart0: uart0 {
> function = "uart0";
> diff --git a/arch/mips/boot/dts/ingenic/cu1000-neo.dts
> b/arch/mips/boot/dts/ingenic/cu1000-neo.dts
> index 22a1066d637b..44d47d12db12 100644
> --- a/arch/mips/boot/dts/ingenic/cu1000-neo.dts
> +++ b/arch/mips/boot/dts/ingenic/cu1000-neo.dts
> @@ -3,7 +3,7 @@
>
> #include "x1000.dtsi"
> #include
> -#include
> +#include
> #include
>
> / {
> @@ -31,6 +31,18 @@
> };
> };
>
> + ssi: spi-gpio {
> + compatible = "spi-gpio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + num-chipselects = <1>;
> +
> + mosi-gpios = < 2 GPIO_ACTIVE_HIGH>;
> + miso-gpios = < 3 GPIO_ACTIVE_HIGH>;
> + sck-gpios = < 0 GPIO_ACTIVE_HIGH>;
> + cs-gpios = < 1 GPIO_ACTIVE_HIGH>;
> + };
> +
> wlan_pwrseq: msc1-pwrseq {
> compatible = "mmc-pwrseq-simple";
>
> @@ -43,13 +55,19 @@
> clock-frequency = <2400>;
> };
>
> - {
> + {
> + /*
> + * Use the 32.768 kHz oscillator as the parent of the RTC for a higher
> + * precision.
> + */
> + assigned-clocks = < X1000_CLK_RTC>;
> + assigned-clock-parents = < X1000_CLK_RTCLK>;
> +};
> +
> + {
> /* 1500 kHz for the system timer and clocksource */
> - assigned-clocks = < TCU_CLK_TIMER0>, < TCU_CLK_TIMER2>;
> + assigned-clocks = < OST_CLK_PERCPU_TIMER>, <
> OST_CLK_GLOBAL_TIMER>;
> assigned-clock-rates = <150>, <150>;
> -
> - /* Use channel #0 for the system timer channel #2 for the clocksource */
> - ingenic,pwm-channels-mask = <0xfa>;
> };
>
> {
> @@ -59,6 +77,32 @@
> pinctrl-0 = <_uart2>;
> };
>
> + {
> + status = "okay";
> +
> + spi-max-frequency = <5000>;
> +
> + sc16is752: expander@0 {
> + compatible = "nxp,sc16is752";
> + reg = <0>; /* CE0 */
> + spi-max-frequency = <400>;
> +
> + clocks = <_sc16is752>;
> +
> + interrupt-parent = <>;
> + interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
> +
> + gpio-controller;
> + #gpio-cells = <2>;
> +
> + exclk_sc16is752: sc16is752 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <4800>;
> + };
> + };
> +};
> +
> {
> status = "okay";
>
> @@ -135,6 +179,10 @@
> };
> };
>
> + {
> + status = "okay";
> +};
> +
> {
> pins_uart2: uart2 {
> function = "uart2";
> diff --git a/arch/mips/boot/dts/ingenic/cu1830-neo.dts
> b/arch/mips/boot/dts/ingenic/cu1830-neo.dts
> index 640f96c00d63..7a56e344e429 100644
> --- a/arch/mips/boot/dts/ingenic/cu1830-neo.dts
> +++ b/arch/mips/boot/dts/ingenic/cu1830-neo.dts
> @@ -3,7 +3,7 @@
>
> #include "x1830.dtsi"
> #include
> -#include
> +#include
> #include
>
> / {
> @@ -31,6 +31,18 @@
> };
> };
>
> + ssi0: spi-gpio {
> + compatible = "spi-gpio";
> + #address-cells =
Re: [PATCH 2/2] MIPS: Ingenic: Refresh defconfig for Ingenic SoCs based boards.
> Am 07.11.2020 um 12:52 schrieb 周琰杰 (Zhou Yanjie) : > > 1.Refresh defconfig of CI20 to support OTG and RNG. > 2.Refresh defconfig of CU1000-Neo to support OTG/RNG/OST/SC16IS752. > 3.Refresh defconfig of CU1830-Neo to support OTG/DTRNG/OST/SC16IS752. > > Tested-by: 周正 (Zhou Zheng) > Signed-off-by: 周琰杰 (Zhou Yanjie) > --- > arch/mips/configs/ci20_defconfig | 14 -- > arch/mips/configs/cu1000-neo_defconfig | 28 +++- > arch/mips/configs/cu1830-neo_defconfig | 32 +--- > 3 files changed, 60 insertions(+), 14 deletions(-) > > diff --git a/arch/mips/configs/ci20_defconfig > b/arch/mips/configs/ci20_defconfig > index 052c5ad0f2b1..80fbe57d68d4 100644 > --- a/arch/mips/configs/ci20_defconfig > +++ b/arch/mips/configs/ci20_defconfig > @@ -49,6 +49,8 @@ CONFIG_MTD_RAW_NAND=y > CONFIG_MTD_NAND_JZ4780=y > CONFIG_MTD_UBI=y > CONFIG_MTD_UBI_FASTMAP=y > +CONFIG_SCSI=y > +CONFIG_BLK_DEV_SD=y > CONFIG_NETDEVICES=y > # CONFIG_NET_VENDOR_ARC is not set > # CONFIG_NET_VENDOR_BROADCOM is not set > @@ -77,7 +79,6 @@ CONFIG_SERIAL_8250_NR_UARTS=5 > CONFIG_SERIAL_8250_RUNTIME_UARTS=5 > CONFIG_SERIAL_8250_INGENIC=y > CONFIG_SERIAL_OF_PLATFORM=y > -# CONFIG_HW_RANDOM is not set > CONFIG_I2C=y > CONFIG_I2C_JZ4780=y > CONFIG_SPI=y > @@ -99,7 +100,12 @@ CONFIG_IR_GPIO_TX=m > CONFIG_MEDIA_SUPPORT=m > # CONFIG_VGA_CONSOLE is not set > # CONFIG_HID is not set > -# CONFIG_USB_SUPPORT is not set > +CONFIG_USB=y > +CONFIG_USB_STORAGE=y > +CONFIG_USB_DWC2=y > +CONFIG_USB_SERIAL=y > +CONFIG_USB_SERIAL_CH341=y > +CONFIG_USB_GADGET=y > CONFIG_MMC=y > CONFIG_MMC_JZ4740=y > CONFIG_NEW_LEDS=y > @@ -131,8 +137,12 @@ CONFIG_MEMORY=y > CONFIG_JZ4780_NEMC=y > CONFIG_PWM=y > CONFIG_PWM_JZ4740=m maybe you can add +CONFIG_JZ4780_EFUSE=y here. It was forgotten when we made the NVRAM for Ethernet working. > +CONFIG_JZ4770_PHY=y > CONFIG_EXT4_FS=y > # CONFIG_DNOTIFY is not set > +CONFIG_AUTOFS_FS=y > +CONFIG_VFAT_FS=y > +CONFIG_FAT_DEFAULT_UTF8=y > CONFIG_PROC_KCORE=y > # CONFIG_PROC_PAGE_MONITOR is not set > CONFIG_TMPFS=y > diff --git a/arch/mips/configs/cu1000-neo_defconfig > b/arch/mips/configs/cu1000-neo_defconfig > index 55d0690a3ffe..9d75f5b77d5d 100644 > --- a/arch/mips/configs/cu1000-neo_defconfig > +++ b/arch/mips/configs/cu1000-neo_defconfig > @@ -25,6 +25,7 @@ CONFIG_HIGHMEM=y > CONFIG_HZ_100=y > # CONFIG_SECCOMP is not set > # CONFIG_SUSPEND is not set > +CONFIG_MODULES=y > # CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set > # CONFIG_COMPACTION is not set > CONFIG_CMA=y > @@ -32,15 +33,17 @@ CONFIG_NET=y > CONFIG_PACKET=y > CONFIG_UNIX=y > CONFIG_INET=y > -CONFIG_CFG80211=y > +CONFIG_CFG80211=m > CONFIG_UEVENT_HELPER=y > CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug" > CONFIG_DEVTMPFS=y > # CONFIG_ALLOW_DEV_COREDUMP is not set > +CONFIG_SCSI=y > +CONFIG_BLK_DEV_SD=y > CONFIG_NETDEVICES=y > CONFIG_STMMAC_ETH=y > CONFIG_SMSC_PHY=y > -CONFIG_BRCMFMAC=y > +CONFIG_BRCMFMAC=m > # CONFIG_INPUT_KEYBOARD is not set > # CONFIG_INPUT_MOUSE is not set > # CONFIG_SERIO is not set > @@ -52,16 +55,25 @@ CONFIG_SERIAL_8250_NR_UARTS=3 > CONFIG_SERIAL_8250_RUNTIME_UARTS=3 > CONFIG_SERIAL_8250_INGENIC=y > CONFIG_SERIAL_OF_PLATFORM=y > -# CONFIG_HW_RANDOM is not set > +CONFIG_SERIAL_SC16IS7XX=y > +# CONFIG_SERIAL_SC16IS7XX_I2C is not set > +CONFIG_SERIAL_SC16IS7XX_SPI=y > CONFIG_I2C=y > CONFIG_I2C_JZ4780=y > +CONFIG_SPI=y > +CONFIG_SPI_GPIO=y > CONFIG_GPIO_SYSFS=y > -CONFIG_SENSORS_ADS7828=y > +CONFIG_SENSORS_ADS7828=m > CONFIG_WATCHDOG=y > CONFIG_JZ4740_WDT=y > # CONFIG_VGA_CONSOLE is not set > # CONFIG_HID is not set > -# CONFIG_USB_SUPPORT is not set > +CONFIG_USB=y > +CONFIG_USB_STORAGE=y > +CONFIG_USB_DWC2=y > +CONFIG_USB_SERIAL=y > +CONFIG_USB_SERIAL_CH341=y > +CONFIG_USB_GADGET=y > CONFIG_MMC=y > CONFIG_MMC_JZ4740=y > CONFIG_NEW_LEDS=y > @@ -72,16 +84,22 @@ CONFIG_RTC_CLASS=y > CONFIG_RTC_DRV_JZ4740=y > CONFIG_DMADEVICES=y > CONFIG_DMA_JZ4780=y > +# CONFIG_INGENIC_TIMER is not set > +CONFIG_INGENIC_SYSOST=y > # CONFIG_IOMMU_SUPPORT is not set > +CONFIG_JZ4770_PHY=y > CONFIG_EXT4_FS=y > # CONFIG_DNOTIFY is not set > CONFIG_AUTOFS_FS=y > +CONFIG_VFAT_FS=y > +CONFIG_FAT_DEFAULT_UTF8=y > CONFIG_PROC_KCORE=y > # CONFIG_PROC_PAGE_MONITOR is not set > CONFIG_TMPFS=y > CONFIG_CONFIGFS_FS=y > CONFIG_NFS_FS=y > CONFIG_NLS=y > +CONFIG_NLS_CODEPAGE_437=y > CONFIG_NLS_CODEPAGE_936=y > CONFIG_NLS_CODEPAGE_950=y > CONFIG_NLS_ASCII=y > diff --git a/arch/mips/configs/cu1830-neo_defconfig > b/arch/mips/configs/cu1830-neo_defconfig > index e7064851a47a..29decd0003c6 100644 > --- a/arch/mips/configs/cu1830-neo_defconfig > +++ b/arch/mips/configs/cu1830-neo_defconfig > @@ -25,6 +25,7 @@ CONFIG_HIGHMEM=y > CONFIG_HZ_100=y > # CONFIG_SECCOMP is not set > # CONFIG_SUSPEND is not set > +CONFIG_MODULES=y > # CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set > # CONFIG_COMPACTION is not set > CONFIG_CMA=y > @@ -32,18 +33,20 @@ CONFIG_NET=y > CONFIG_PACKET=y > CONFIG_UNIX=y > CONFIG_INET=y > -CONFIG_CFG80211=y
Re: [Letux-kernel] [REGRESSION] opp: Allow dev_pm_opp_get_opp_table() to return -EPROBE_DEFER
Hi,
> Am 06.11.2020 um 09:58 schrieb Viresh Kumar :
>
> On 06-11-20, 09:44, H. Nikolaus Schaller wrote:
>>
>>> Am 06.11.2020 um 05:14 schrieb Viresh Kumar :
>>>
>>> On 06-11-20, 00:10, Andreas Kemnade wrote:
>>>> Hi,
>>>>
>>>> On the GTA04 (DM3730, devicetree omap3-gta04*) I get my console flooded
>>>> up with the following:
>>>> [ 24.517211] cpu cpu0: multiple regulators are not supported
>>>> [ 24.523040] cpufreq: __target_index: Failed to change cpu frequency: -22
>>>> [ 24.537231] [ cut here ]
>>>> [ 24.542083] WARNING: CPU: 0 PID: 5 at drivers/opp/core.c:678
>>>> dev_pm_opp_set_rate+0x23c/0x494
>>>> [ 24.551086] Modules linked in: usb_f_ecm g_ether usb_f_rndis u_ether
>>>> libcomposite configfs phy_twl4030_usb omap2430 musb_hdrc overlay
>>>> [ 24.563842] CPU: 0 PID: 5 Comm: kworker/0:0 Tainted: GW
>>>> 5.9.0-rc1-8-g629238068eb9 #14
>>>> [ 24.573852] Hardware name: Generic OMAP36xx (Flattened Device Tree)
>>>> [ 24.580413] Workqueue: events dbs_work_handler
>>>> [ 24.585083] [] (unwind_backtrace) from []
>>>> (show_stack+0x10/0x14)
>>>> [ 24.593200] [] (show_stack) from []
>>>> (dump_stack+0x8c/0xac)
>>>> [ 24.600769] [] (dump_stack) from []
>>>> (__warn+0xcc/0xe4)
>>>> [ 24.608001] [] (__warn) from []
>>>> (warn_slowpath_fmt+0x74/0xa0)
>>>> [ 24.615844] [] (warn_slowpath_fmt) from []
>>>> (dev_pm_opp_set_rate+0x23c/0x494)
>>>> [ 24.625061] [] (dev_pm_opp_set_rate) from []
>>>> (set_target+0x2c/0x4c)
>>>> [ 24.633453] [] (set_target) from []
>>>> (__cpufreq_driver_target+0x190/0x22c)
>>>> [ 24.642395] [] (__cpufreq_driver_target) from []
>>>> (od_dbs_update+0xcc/0x158)
>>>> [ 24.651489] [] (od_dbs_update) from []
>>>> (dbs_work_handler+0x2c/0x54)
>>>> [ 24.659881] [] (dbs_work_handler) from []
>>>> (process_one_work+0x210/0x358)
>>>> [ 24.668731] [] (process_one_work) from []
>>>> (worker_thread+0x22c/0x2d0)
>>>> [ 24.677307] [] (worker_thread) from []
>>>> (kthread+0x140/0x14c)
>>>> [ 24.685058] [] (kthread) from []
>>>> (ret_from_fork+0x14/0x2c)
>>>> [ 24.692626] Exception stack(0xde4b7fb0 to 0xde4b7ff8)
>>>> [ 24.697906] 7fa0:
>>>>
>>>> [ 24.706481] 7fc0:
>>>>
>>>> [ 24.715057] 7fe0: 0013
>>>> [ 24.722198] ---[ end trace 038b3f231fae6f81 ]---
>>>>
>>>> endlessly after the $subject commit. Any hints?
>>>
>>> The fix for this has been in linux-next for a couple of days and it
>>> made it to linus/master yesterday.
>>>
>>> 47efcbcb340ic opp: Fix early exit from dev_pm_opp_register_set_opp_helper()
>
> I think I may have accidentally pasted the wrong commit here. This is
> the one which must have fixed it for you.
>
> commit 1f6620f87006 ("opp: Don't always remove static OPPs in
> _of_add_opp_table_v1()")
Well, I did a cross-check and git revert 47efcbcb340 made the problem come back.
Maybe both patches are good and the first one hides the missing second one.
What I haven't checked is if all opps are available now. I just looked for the
omap to boot.
>
>
>> Seems to fix our problems on gta04 (OMAP3).
>> Otherwise we would have found that v5.10-rc3 magically solves it :)
>
> I assume you just ran linus's/master, otherwise the patch I shared
> earlier won't have fixed the issue :)
Yes, we just test with v5.10-rc2 and wait for -rc3 to come in some days.
>
>> Interestingly it did not affect OMAP5.
>
> Based on the DT I saw for omap5, it does use OPPv1 and so it shouldn't
> have worked as well. It may be worth checking why it didn't get
> affected earlier.
>
> You can see the populated OPPs for a platform with this:
>
> ls /sys/kernel/debug/opp/cpu*/*
>
> You shall see some difference with and without this patch. Or it may
> be the case that you are adding dynamic OPPs with dev_pm_opp_add() and
> so even after removing the static ones, it worked (though I wasn't
> able to find that in the code).
Ah, now as you tell this I remember that the last test on omap5 did not
have any cpufreq info output. Although it did boot to login:.
So I did not see a common reason in these quite different symptoms.
I am sure that with -rc3 omap3 & omap5 will be ok again and I'll take a
special look at it when testing other things.
BR and thanks,
Nikolaus
Re: [Letux-kernel] [REGRESSION] opp: Allow dev_pm_opp_get_opp_table() to return -EPROBE_DEFER
> Am 06.11.2020 um 05:14 schrieb Viresh Kumar : > > On 06-11-20, 00:10, Andreas Kemnade wrote: >> Hi, >> >> On the GTA04 (DM3730, devicetree omap3-gta04*) I get my console flooded >> up with the following: >> [ 24.517211] cpu cpu0: multiple regulators are not supported >> [ 24.523040] cpufreq: __target_index: Failed to change cpu frequency: -22 >> [ 24.537231] [ cut here ] >> [ 24.542083] WARNING: CPU: 0 PID: 5 at drivers/opp/core.c:678 >> dev_pm_opp_set_rate+0x23c/0x494 >> [ 24.551086] Modules linked in: usb_f_ecm g_ether usb_f_rndis u_ether >> libcomposite configfs phy_twl4030_usb omap2430 musb_hdrc overlay >> [ 24.563842] CPU: 0 PID: 5 Comm: kworker/0:0 Tainted: GW >> 5.9.0-rc1-8-g629238068eb9 #14 >> [ 24.573852] Hardware name: Generic OMAP36xx (Flattened Device Tree) >> [ 24.580413] Workqueue: events dbs_work_handler >> [ 24.585083] [] (unwind_backtrace) from [] >> (show_stack+0x10/0x14) >> [ 24.593200] [] (show_stack) from [] >> (dump_stack+0x8c/0xac) >> [ 24.600769] [] (dump_stack) from [] (__warn+0xcc/0xe4) >> [ 24.608001] [] (__warn) from [] >> (warn_slowpath_fmt+0x74/0xa0) >> [ 24.615844] [] (warn_slowpath_fmt) from [] >> (dev_pm_opp_set_rate+0x23c/0x494) >> [ 24.625061] [] (dev_pm_opp_set_rate) from [] >> (set_target+0x2c/0x4c) >> [ 24.633453] [] (set_target) from [] >> (__cpufreq_driver_target+0x190/0x22c) >> [ 24.642395] [] (__cpufreq_driver_target) from [] >> (od_dbs_update+0xcc/0x158) >> [ 24.651489] [] (od_dbs_update) from [] >> (dbs_work_handler+0x2c/0x54) >> [ 24.659881] [] (dbs_work_handler) from [] >> (process_one_work+0x210/0x358) >> [ 24.668731] [] (process_one_work) from [] >> (worker_thread+0x22c/0x2d0) >> [ 24.677307] [] (worker_thread) from [] >> (kthread+0x140/0x14c) >> [ 24.685058] [] (kthread) from [] >> (ret_from_fork+0x14/0x2c) >> [ 24.692626] Exception stack(0xde4b7fb0 to 0xde4b7ff8) >> [ 24.697906] 7fa0: >> >> [ 24.706481] 7fc0: >> >> [ 24.715057] 7fe0: 0013 >> [ 24.722198] ---[ end trace 038b3f231fae6f81 ]--- >> >> endlessly after the $subject commit. Any hints? > > The fix for this has been in linux-next for a couple of days and it > made it to linus/master yesterday. > > 47efcbcb340c opp: Fix early exit from dev_pm_opp_register_set_opp_helper() Seems to fix our problems on gta04 (OMAP3). Otherwise we would have found that v5.10-rc3 magically solves it :) Interestingly it did not affect OMAP5. BR and thanks, Nikolaus Schaller
Re: [PATCH 0/2] AMR: DTS: fix and extension for Pandaboard ES
ping > Am 03.10.2020 um 16:09 schrieb H. Nikolaus Schaller : > > * fix wrong pinmux offset preventing the user button from working > * add uart connection for bluetooth wl1271 hci > > H. Nikolaus Schaller (2): > ARM: dts: pandaboard: fix pinmux for gpio user button of Pandaboard ES > ARM: dts: pandaboard es: add bluetooth uart for HCI > > arch/arm/boot/dts/omap4-panda-es.dts | 34 +++- > 1 file changed, 33 insertions(+), 1 deletion(-) > > -- > 2.26.2 >
[PATCH 0/2] AMR: DTS: fix and extension for Pandaboard ES
* fix wrong pinmux offset preventing the user button from working * add uart connection for bluetooth wl1271 hci H. Nikolaus Schaller (2): ARM: dts: pandaboard: fix pinmux for gpio user button of Pandaboard ES ARM: dts: pandaboard es: add bluetooth uart for HCI arch/arm/boot/dts/omap4-panda-es.dts | 34 +++- 1 file changed, 33 insertions(+), 1 deletion(-) -- 2.26.2
[PATCH 2/2] ARM: dts: pandaboard es: add bluetooth uart for HCI
The wl271 bluetooth uart is connected to uart2.
Setup a serdev uart child and separate bluetooth and uart2 pinmux
from wl12xx pinmux to better group the pins and muxes.
Signed-off-by: H. Nikolaus Schaller
---
arch/arm/boot/dts/omap4-panda-es.dts | 32
1 file changed, 32 insertions(+)
diff --git a/arch/arm/boot/dts/omap4-panda-es.dts
b/arch/arm/boot/dts/omap4-panda-es.dts
index 6afa8fd7c412de..7c6886cd738f03 100644
--- a/arch/arm/boot/dts/omap4-panda-es.dts
+++ b/arch/arm/boot/dts/omap4-panda-es.dts
@@ -49,6 +49,22 @@ button_pins: pinmux_button_pins {
OMAP4_IOPAD(0x0fc, PIN_INPUT_PULLUP | MUX_MODE3) /*
gpio_113 */
>;
};
+
+ bt_pins: pinmux_bt_pins {
+ pinctrl-single,pins = <
+ OMAP4_IOPAD(0x06c, PIN_OUTPUT | MUX_MODE3)
/* gpmc_a22.gpio_46 - BTEN */
+ OMAP4_IOPAD(0x072, PIN_OUTPUT_PULLUP | MUX_MODE3)
/* gpmc_a25.gpio_49 - BTWAKEUP */
+ >;
+ };
+
+ uart2_pins: pinmux_uart2_pins {
+ pinctrl-single,pins = <
+ OMAP4_IOPAD(0x118, PIN_INPUT_PULLUP | MUX_MODE0)
/* uart2_cts.uart2_cts - HCI */
+ OMAP4_IOPAD(0x11a, PIN_OUTPUT | MUX_MODE0)
/* uart2_rts.uart2_rts */
+ OMAP4_IOPAD(0x11c, PIN_INPUT_PULLUP | MUX_MODE0)
/* uart2_rx.uart2_rx */
+ OMAP4_IOPAD(0x11e, PIN_OUTPUT | MUX_MODE0)
/* uart2_tx.uart2_tx */
+ >;
+ };
};
_wkgpio_pins {
@@ -80,3 +96,19 @@ buttonS2 {
_target {
ti,no-reset-on-init;
};
+
+_gpio {
+ pinctrl-single,pins = <
+ OMAP4_IOPAD(0x066, PIN_OUTPUT | MUX_MODE3) /*
gpmc_a19.gpio_43 */
+ OMAP4_IOPAD(0x070, PIN_OUTPUT_PULLUP | MUX_MODE3) /*
gpmc_a24.gpio_48 */
+ >;
+};
+
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins _pins>;
+ bluetooth: tiwi {
+ compatible = "ti,wl1271-st";
+ enable-gpios = < 14 GPIO_ACTIVE_HIGH>;/* GPIO_46 */
+ };
+};
--
2.26.2
[PATCH 1/2] ARM: dts: pandaboard: fix pinmux for gpio user button of Pandaboard ES
The pinmux control register offset passed to OMAP4_IOPAD is odd.
Fixes: ab9a13665e7c ("ARM: dts: pandaboard: add gpio user button")
Cc: sta...@vger.kernel.org
Signed-off-by: H. Nikolaus Schaller
---
arch/arm/boot/dts/omap4-panda-es.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap4-panda-es.dts
b/arch/arm/boot/dts/omap4-panda-es.dts
index cfa85aa3da085e..6afa8fd7c412de 100644
--- a/arch/arm/boot/dts/omap4-panda-es.dts
+++ b/arch/arm/boot/dts/omap4-panda-es.dts
@@ -46,7 +46,7 @@ OMAP4_IOPAD(0x0f6, PIN_OUTPUT | MUX_MODE3)/* gpio_110 */
button_pins: pinmux_button_pins {
pinctrl-single,pins = <
- OMAP4_IOPAD(0x11b, PIN_INPUT_PULLUP | MUX_MODE3) /*
gpio_113 */
+ OMAP4_IOPAD(0x0fc, PIN_INPUT_PULLUP | MUX_MODE3) /*
gpio_113 */
>;
};
};
--
2.26.2
Re: [PATCH v8 5/6] MIPS: Ingenic: Add 'cpus' node for Ingenic SoCs.
Hi Zhou Yanjie,
what is the status of this series? It does not seem to have arrived in
linux-next for v5.10-rc1.
BR and thanks,
Nikolaus
> Am 19.05.2020 um 16:35 schrieb 周琰杰 (Zhou Yanjie) :
>
> Add 'cpus' node to the jz4740.dtsi, jz4770.dtsi, jz4780.dtsi
> and x1000.dtsi files.
>
> Tested-by: H. Nikolaus Schaller
> Tested-by: Paul Boddie
> Signed-off-by: 周琰杰 (Zhou Yanjie)
> ---
>
> Notes:
>v1->v2:
>No change.
>
>v2->v3:
>No change.
>
>v3->v4:
>Rebase on top of kernel 5.6-rc1.
>
>v4->v5:
>No change.
>
>v5->v6:
>No change.
>
>v6->v7:
>Update compatible strings.
>
>v7->v8:
>No change.
>
> arch/mips/boot/dts/ingenic/jz4740.dtsi | 14 ++
> arch/mips/boot/dts/ingenic/jz4770.dtsi | 15 ++-
> arch/mips/boot/dts/ingenic/jz4780.dtsi | 23 +++
> arch/mips/boot/dts/ingenic/x1000.dtsi | 14 ++
> 4 files changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi
> b/arch/mips/boot/dts/ingenic/jz4740.dtsi
> index a3301ba..1f2f896 100644
> --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
> @@ -7,6 +7,20 @@
> #size-cells = <1>;
> compatible = "ingenic,jz4740";
>
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "ingenic,xburst-mxu1.0";
> + reg = <0>;
> +
> + clocks = < JZ4740_CLK_CCLK>;
> + clock-names = "cpu";
> + };
> + };
> +
> cpuintc: interrupt-controller {
> #address-cells = <0>;
> #interrupt-cells = <1>;
> diff --git a/arch/mips/boot/dts/ingenic/jz4770.dtsi
> b/arch/mips/boot/dts/ingenic/jz4770.dtsi
> index 0bfb9ed..12c7101 100644
> --- a/arch/mips/boot/dts/ingenic/jz4770.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4770.dtsi
> @@ -1,5 +1,4 @@
> // SPDX-License-Identifier: GPL-2.0
> -
> #include
>
> / {
> @@ -7,6 +6,20 @@
> #size-cells = <1>;
> compatible = "ingenic,jz4770";
>
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "ingenic,xburst-fpu1.0-mxu1.1";
> + reg = <0>;
> +
> + clocks = < JZ4770_CLK_CCLK>;
> + clock-names = "cpu";
> + };
> + };
> +
> cpuintc: interrupt-controller {
> #address-cells = <0>;
> #interrupt-cells = <1>;
> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> index bb89653..03aeeff 100644
> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> @@ -8,6 +8,29 @@
> #size-cells = <1>;
> compatible = "ingenic,jz4780";
>
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "ingenic,xburst-fpu1.0-mxu1.1";
> + reg = <0>;
> +
> + clocks = < JZ4780_CLK_CPU>;
> + clock-names = "cpu";
> + };
> +
> + cpu1: cpu@1 {
> + device_type = "cpu";
> + compatible = "ingenic,xburst-fpu1.0-mxu1.1";
> + reg = <1>;
> +
> + clocks = < JZ4780_CLK_CORE1>;
> + clock-names = "cpu";
> + };
> + };
> +
> cpuintc: interrupt-controller {
> #address-cells = <0>;
> #interrupt-cells = <1>;
> diff --git a/arch/mips/boot/dts/ingenic/x1000.dtsi
> b/arch/mips/boot/dts/ingenic/x1000.dtsi
> index 147f7d5..2205e1b 100644
> --- a/arch/mips/boot/dts/ingenic/x1000.dtsi
> +++ b/arch/mips/boot/dts/ingenic/x1000.dtsi
> @@ -8,6 +8,20 @@
> #size-cells = <1>;
> compatible = "ingenic,x1000", "ingenic,x1000e";
>
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "ingenic,xburst-fpu1.0-mxu1.1";
> + reg = <0>;
> +
> + clocks = < X1000_CLK_CPU>;
> + clock-names = "cpu";
> + };
> + };
> +
> cpuintc: interrupt-controller {
> #address-cells = <0>;
> #interrupt-cells = <1>;
> --
> 2.7.4
>
Re: Mailing list about low levels of Linux on cellphones
Hi Pavel, > Am 09.09.2020 um 00:56 schrieb Pavel Machek : > > Hi! > > It seems there is quite a lot of efforts porting kernel to various > cellphones. Yes, and it is sisyphos work to get it 100% into mainline... Most projects I know about are at 99,5%. > > Librem 5 and PinePhone have their own hardware, people around Maemo > Leste work with Nokia N900 and Droid 4, there's group working with > Sony cellphones, there are postmarketOS people and there are probably > groups I don't know about. And there is Openmoko and successors (GTA04, Pyra, GTA15). > I believe some coordination would be useful, so we end up with > compatible solutions for various problems. IMHO the main problem we all share is power management. > > It would be also good to know how ther hardware is progressing. I'd > really like to have phone I could use as a _phone_, running mainline > kernel. So far N900 with original Maemo is closest I could get. > > Would it be possible to create a mailing list on vger.kernel.org? Hm. My experience with asking for creating an openpvrsgx mailing list at vger.kernel.org was like asking the sun to shine during night. In the end we did set up our own mailing list and github project: https://lists.openphoenux.org/mailman/listinfo.cgi/openpvrsgx-devgroup https://github.com/openpvrsgx-devgroup/linux_openpvrsgx What I can offer is to run a similar list. You may remember that OpenPhoenux was the continuation of the OpenMoko community mailing lists having the same focus as you describe above. > Probably phones@ or phone-devel@? I believe it would be useful to > cover hardware-dependend pieces of the phone stack (ofono, > modemmanager) as well as kernel. I am fine with either. BR, Nikolaus
[BUG]: KVM: arm64: Fix symbol dependency in __hyp_call_panic_nvhe
Hi,
it seems as if your patch
34f379956e9d7 ("KVM: arm64: Fix symbol dependency in __hyp_call_panic_nvhe")
[ Upstream commit b38b298aa4397e2dc74a89b4dd3eac9e59b64c96 ]
fails to compile in v5.8.7 for me (using an aarch64 gcc 4.9 cross-toolchain to
try
to build a kernel for the PinePhone):
CC arch/arm64/kvm/hyp/switch.o - due to target missing
arch/arm64/kvm/hyp/switch.c: In function 'hyp_panic':
arch/arm64/kvm/hyp/switch.c:904:2: error: impossible constraint in 'asm'
asm volatile("ldr %0, =%1" : "=r" (str_va) : "S" (__hyp_panic_string));
^
I can find the commit b38b298aa4397e2dc74a89b4dd3eac9e59b64c96 in upstream
but not the affected file. There is also "KVM: arm64: Split hyp/switch.c to
VHE/nVHE"
which does a cleanup and rename and v5.9-rc4 compiles fine.
With a git revert 34f379956e9d7 on v5.8.7 I can compile without problems.
So something seems to be incomplete or premature with the backport.
BR and thanks,
Nikolaus Schaller
Re: [BUG]: KVM: arm64: Fix symbol dependency in __hyp_call_panic_nvhe
Hi,
> Am 07.09.2020 um 16:22 schrieb Sasha Levin :
>
> On Mon, Sep 07, 2020 at 03:29:40PM +0200, H. Nikolaus Schaller wrote:
>> Hi,
>> it seems as if your patch
>>
>> 34f379956e9d7 ("KVM: arm64: Fix symbol dependency in __hyp_call_panic_nvhe")
>> [ Upstream commit b38b298aa4397e2dc74a89b4dd3eac9e59b64c96 ]
>>
>> fails to compile in v5.8.7 for me (using an aarch64 gcc 4.9 cross-toolchain
>> to try
>> to build a kernel for the PinePhone):
>>
>> CC arch/arm64/kvm/hyp/switch.o - due to target missing
>> arch/arm64/kvm/hyp/switch.c: In function 'hyp_panic':
>> arch/arm64/kvm/hyp/switch.c:904:2: error: impossible constraint in 'asm'
>> asm volatile("ldr %0, =%1" : "=r" (str_va) : "S" (__hyp_panic_string));
>> ^
>
> Does upstream build correctly for you?
It is 100% upstream code in arch/arm64, just with a private config.
diff --stat arch/arm64 shows only 2 dts and 2 config files. It did
compile well with v5.8.5 and just broke after merge v5.8.7.
>
>> I can find the commit b38b298aa4397e2dc74a89b4dd3eac9e59b64c96 in upstream
>> but not the affected file. There is also "KVM: arm64: Split hyp/switch.c to
>> VHE/nVHE"
>> which does a cleanup and rename and v5.9-rc4 compiles fine.
>
> Right, it got moved around in upstream.
Maybe this has fixed something...
BR and thanks,
Nikolaus
Re: [Letux-kernel] [PATCH] omap5: Fix DSI base address and clocks
> Am 18.08.2020 um 11:51 schrieb David Shah :
>
> DSI was not probing due to base address off by 0x1000, and sys_clk
> missing.
>
> With this patch, the Pyra display works if HDMI is disabled in the
> device tree.
For me it also works if HDMI is not disabled.
So IMHO this comment is misleading.
Otherwise,
Tested-by: H. Nikolaus Schaller
>
> Signed-off-by: David Shah
> ---
> arch/arm/boot/dts/omap5.dtsi | 20 +++-
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index c96e19a15c52..849a2dd9fef7 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -388,11 +388,11 @@ rfbi: encoder@0 {
> };
> };
>
> - target-module@5000 {
> + target-module@4000 {
> compatible = "ti,sysc-omap2", "ti,sysc";
> - reg = <0x5000 0x4>,
> - <0x5010 0x4>,
> - <0x5014 0x4>;
> + reg = <0x4000 0x4>,
> + <0x4010 0x4>,
> + <0x4014 0x4>;
> reg-names = "rev", "sysc", "syss";
> ti,sysc-sidle = ,
> ,
> @@ -404,7 +404,7 @@ SYSC_OMAP2_SOFTRESET |
> ti,syss-mask = <1>;
> #address-cells = <1>;
> #size-cells = <1>;
> - ranges = <0 0x5000 0x1000>;
> + ranges = <0 0x4000 0x1000>;
>
> dsi1: encoder@0 {
> compatible = "ti,omap5-dsi";
> @@ -414,8 +414,9 @@ dsi1: encoder@0 {
> reg-names = "proto", "phy",
> "pll";
> interrupts = IRQ_TYPE_LEVEL_HIGH>;
> status = "disabled";
> - clocks = <_clkctrl
> OMAP5_DSS_CORE_CLKCTRL 8>;
> - clock-names = "fck";
> + clocks = <_clkctrl
> OMAP5_DSS_CORE_CLKCTRL 8>,
> + <_clkctrl
> OMAP5_DSS_CORE_CLKCTRL 10>;
> + clock-names = "fck", "sys_clk";
> };
> };
>
> @@ -445,8 +446,9 @@ dsi2: encoder@0 {
> reg-names = "proto", "phy",
> "pll";
> interrupts = IRQ_TYPE_LEVEL_HIGH>;
> status = "disabled";
> - clocks = <_clkctrl
> OMAP5_DSS_CORE_CLKCTRL 8>;
> - clock-names = "fck";
> + clocks = <_clkctrl
> OMAP5_DSS_CORE_CLKCTRL 8>,
> + <_clkctrl
> OMAP5_DSS_CORE_CLKCTRL 10>;
> + clock-names = "fck", "sys_clk";
> };
> };
>
> --
> 2.28.0
>
> ___
> https://projects.goldelico.com/p/gta04-kernel/
> Letux-kernel mailing list
> letux-ker...@openphoenux.org
> http://lists.goldelico.com/mailman/listinfo.cgi/letux-kernel
Re: BUG: omap5: v5.8-rc7 boot fails
Hi Tony,
> Am 29.07.2020 um 13:55 schrieb Tony Lindgren :
>
> * Tony Lindgren [200728 08:23]:
>> * H. Nikolaus Schaller [200727 20:51]:
>>> Hi Tony,
>>> after trying v5.8-rc7 the Pyra boot hangs after ca. 3 seconds
>>> (a little random what the last log line is).
>>>
>>> I could bisect it to:
>>>
>>> 6cfcd5563b4fadbf49ba8fa481978e5e86d30322 is the first bad commit
>>> commit 6cfcd5563b4fadbf49ba8fa481978e5e86d30322
>>> Author: Tony Lindgren
>>> Date: Mon Jul 13 09:26:01 2020 -0700
>>>
>>>clocksource/drivers/timer-ti-dm: Fix suspend and resume for am3 and am4
>>>
>>> And a git revert makes it boot again.
>>>
>>> I haven't had time to do more tests (e.g. with omap3/4 or on omap5uevm).
>>
>> Oops sorry about that, I'll take a look.
>
> This fixes booting for me,
yes, I can confirm that this fixes the omap5 Pyra.
And there seems to be no regression on dm3730 GTA04 (but I didn't notice the
bug either, although it also uses a pwm_dmtimer).
BR and thanks,
Nikolaus
> but I still need to check if we also
> need to enable before the reset. And then this needs to be tested
> on all the related SoCs again.
>
> Regards,
>
> Tony
>
> 8< --
> diff --git a/drivers/clocksource/timer-ti-dm-systimer.c
> b/drivers/clocksource/timer-ti-dm-systimer.c
> --- a/drivers/clocksource/timer-ti-dm-systimer.c
> +++ b/drivers/clocksource/timer-ti-dm-systimer.c
> @@ -409,8 +409,8 @@ static int __init dmtimer_systimer_setup(struct
> device_node *np,
> t->wakeup = regbase + _OMAP_TIMER_WAKEUP_EN_OFFSET;
> t->ifctrl = regbase + _OMAP_TIMER_IF_CTRL_OFFSET;
>
> - dmtimer_systimer_enable(t);
> dmtimer_systimer_reset(t);
> + dmtimer_systimer_enable(t);
> pr_debug("dmtimer rev %08x sysc %08x\n", readl_relaxed(t->base),
>readl_relaxed(t->base + t->sysc));
>
BUG: omap5: v5.8-rc7 boot fails
Hi Tony, after trying v5.8-rc7 the Pyra boot hangs after ca. 3 seconds (a little random what the last log line is). I could bisect it to: 6cfcd5563b4fadbf49ba8fa481978e5e86d30322 is the first bad commit commit 6cfcd5563b4fadbf49ba8fa481978e5e86d30322 Author: Tony Lindgren Date: Mon Jul 13 09:26:01 2020 -0700 clocksource/drivers/timer-ti-dm: Fix suspend and resume for am3 and am4 And a git revert makes it boot again. I haven't had time to do more tests (e.g. with omap3/4 or on omap5uevm). BR and thanks, Nikolaus
Re: [PATCH] MIPS: CI20: DTS: Correcting IW8103 Wifi binding
Hi Alexandre,
> Am 06.07.2020 um 22:22 schrieb Alexandre GRIVEAUX :
>
> Le 06/07/2020 à 13:15, H. Nikolaus Schaller a écrit :
>> Hi Alexandre,
>>
>>> Am 05.07.2020 um 12:32 schrieb agrive...@deutnet.info:
>>>
>>> From: Alexandre GRIVEAUX
>>>
>>> Use brcm,bcm4329-fmac instead of brcm,bcm4330-fmac.
>>>
>>> Signed-off-by: Alexandre GRIVEAUX
>>> ---
>>> arch/mips/boot/dts/ingenic/ci20.dts | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts
>>> b/arch/mips/boot/dts/ingenic/ci20.dts
>>> index 75f5bfbf2c37..82a1f126b778 100644
>>> --- a/arch/mips/boot/dts/ingenic/ci20.dts
>>> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
>>> @@ -116,8 +116,8 @@
>>> pinctrl-0 = <_mmc1>;
>>>
>>> brcmf: wifi@1 {
>>> -/* reg = <4>;*/
>>> - compatible = "brcm,bcm4330-fmac";
>>> + reg = <1>;
>>> + compatible = "brcm,bcm4329-fmac";
>>> vcc-supply = <_power>;
>>> device-wakeup-gpios = < 9 GPIO_ACTIVE_HIGH>;
>>> shutdown-gpios = < 7 GPIO_ACTIVE_LOW>;
>> Do you have it working with a v5.8 kernel?
>>
>> I don't see any activity to detect the module or load firmware.
>>
>> Does it rely on some other patch?
>>
>> BR and thanks,
>> Nikolaus
>>
> Hi Nikolaus
>
>
> At this time the patch have been only "tested" for error will doing make:
>
> make ARCH=mips CROSS_COMPILE=mipsel-linux-gnu- olddefconfig && make
> ARCH=mips CROSS_COMPILE=mipsel-linux-gnu- -j8 && make ARCH=mips
> CROSS_COMPILE=mipsel-linux-gnu- -j8 uImage
>
>
> The .config come from creator-ci20 kernel 'config-3.18.3-ci20-1'
>
>
> Even with the right DT
> (Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt)
> it's need some config with brcm enabled I gess.
>
>
> I need to do some investigation will trying the uImage this week,
> unfortunaly kernel developpement is not my main work, it's a hobby.
No problem. For me, the CI20 is also a hobby project :)
Here is some information about the CI20 WiFi and firmware:
https://elinux.org/CI20_Hardware#WiFi.2FBT
https://elinux.org/CI20_upstream#WiFi_firmware
So to be it looks as if the
compatible = "brcm,bcm4330-fmac";
is correct. But the reg = <4> or <1> is something we have to find out.
BR,
Nikolaus
Re: [PATCH] [stable v5.4.x] pwm: jz4740: Fix build failure
> Am 10.07.2020 um 12:27 schrieb Uwe Kleine-König
> :
>
> When commit 9017dc4fbd59 ("pwm: jz4740: Enhance precision in calculation
> of duty cycle") from v5.8-rc1 was backported to v5.4.x its dependency on
> commit ce1f9cece057 ("pwm: jz4740: Use clocks from TCU driver") was not
> noticed which made the pwm-jz4740 driver fail to build.
Please can you add my "reported by?"
> As ce1f9cece057 depends on still more rework, just backport a small part
> of this commit to make the driver build again. (There is no dependency
> on the functionality introduced in ce1f9cece057, just the rate variable
> is needed.)
BR and thanks,
Nikolaus
>
> Signed-off-by: Uwe Kleine-König
> ---
> Hello,
>
> @Paul: Can you please check this is correct? I only build-tested this
> change.
>
> Best regards
> Uwe
>
> drivers/pwm/pwm-jz4740.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
> index d0f5c69930d0..77c28313e95f 100644
> --- a/drivers/pwm/pwm-jz4740.c
> +++ b/drivers/pwm/pwm-jz4740.c
> @@ -92,11 +92,12 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct
> pwm_device *pwm,
> {
> struct jz4740_pwm_chip *jz4740 = to_jz4740(pwm->chip);
> unsigned long long tmp;
> - unsigned long period, duty;
> + unsigned long rate, period, duty;
> unsigned int prescaler = 0;
> uint16_t ctrl;
>
> - tmp = (unsigned long long)clk_get_rate(jz4740->clk) * state->period;
> + rate = clk_get_rate(jz4740->clk);
> + tmp = rate * state->period;
> do_div(tmp, 10);
> period = tmp;
>
> --
> 2.27.0
>
Re: [PATCH] Revert "pwm: jz4740: Enhance precision in calculation of duty cycle"
> Am 10.07.2020 um 12:18 schrieb Uwe Kleine-König
> :
>
> On Fri, Jul 10, 2020 at 09:24:45AM +0200, H. Nikolaus Schaller wrote:
>> This reverts commit a6030d71e62d3e0e270bf3b7fb48d32a636732db.
>>
>> which was applied to v5.4.49. This ends in a compile issue:
>>
>> CC drivers/pwm/pwm-jz4740.o - due to target missing
>> drivers/pwm/pwm-jz4740.c: In function 'jz4740_pwm_apply':
>> drivers/pwm/pwm-jz4740.c:111:28: error: 'rate' undeclared (first use in this
>> function)
>> tmp = (unsigned long long)rate * state->duty_cycle;
>>^
>> drivers/pwm/pwm-jz4740.c:111:28: note: each undeclared identifier is
>> reported only once for each function it appears in
>> make[4]: *** [drivers/pwm/pwm-jz4740.o] Error 1
>>
>> v5.5 and later include the required additional patches to define
>> the rate variable.
>
> So 9017dc4fbd59 ("pwm: jz4740: Enhance precision in calculation of duty
> cycle") which is in v5.8-rc1 was backported to stable:
>
> v5.4.49 (as commit a6030d71e62d3e0e270bf3b7fb48d32a636732db)
> v5.7.5 (as commit e0e71bb7852142a18fb829da419013bb6da9ed3f)
>
> However 9017dc4fbd59 depends on
>
> ce1f9cece057 ("pwm: jz4740: Use clocks from TCU driver")
>
> (which in mainline comes before 9017dc4fbd59 as it's included in
> v5.7-rc1).
>
> As ce1f9cece057 was not backported to v5.4.x, this must either be done, or
> we need to patch that. Will reply with a suggested change.
That is what I did suspect that some other patch this one depends on
was not backported.
What the better strategy (backport missing parts or revert) depends
on how easy it is to backport to v5.4.y.
I am happy with either solution. It is just simpler for me to post
my workaround for the compile issue.
>
> In v5.7.x there is no problem.
In v5.5 and v5.6 there is also no problem. Just v5.4 starting with v5.4.49.
BR and thanks,
Nikolaus
[PATCH] Revert "pwm: jz4740: Enhance precision in calculation of duty cycle"
This reverts commit a6030d71e62d3e0e270bf3b7fb48d32a636732db.
which was applied to v5.4.49. This ends in a compile issue:
CC drivers/pwm/pwm-jz4740.o - due to target missing
drivers/pwm/pwm-jz4740.c: In function 'jz4740_pwm_apply':
drivers/pwm/pwm-jz4740.c:111:28: error: 'rate' undeclared (first use in this
function)
tmp = (unsigned long long)rate * state->duty_cycle;
^
drivers/pwm/pwm-jz4740.c:111:28: note: each undeclared identifier is reported
only once for each function it appears in
make[4]: *** [drivers/pwm/pwm-jz4740.o] Error 1
v5.5 and later include the required additional patches to define
the rate variable.
Fixes: a6030d71e62d ("pwm: jz4740: Enhance precision in calculation of duty
cycle")
Cc: sta...@vger.kernel.org # v5.4.49
Signed-off-by: H. Nikolaus Schaller
---
drivers/pwm/pwm-jz4740.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
index d0f5c69930d0d9..9d78cc21cb1279 100644
--- a/drivers/pwm/pwm-jz4740.c
+++ b/drivers/pwm/pwm-jz4740.c
@@ -108,8 +108,8 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct
pwm_device *pwm,
if (prescaler == 6)
return -EINVAL;
- tmp = (unsigned long long)rate * state->duty_cycle;
- do_div(tmp, NSEC_PER_SEC);
+ tmp = (unsigned long long)period * state->duty_cycle;
+ do_div(tmp, state->period);
duty = period - tmp;
if (duty >= period)
--
2.26.2
Re: [PATCH] MIPS: CI20: DTS: Correcting IW8103 Wifi binding
Hi Alexandre,
> Am 05.07.2020 um 12:32 schrieb agrive...@deutnet.info:
>
> From: Alexandre GRIVEAUX
>
> Use brcm,bcm4329-fmac instead of brcm,bcm4330-fmac.
>
> Signed-off-by: Alexandre GRIVEAUX
> ---
> arch/mips/boot/dts/ingenic/ci20.dts | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts
> b/arch/mips/boot/dts/ingenic/ci20.dts
> index 75f5bfbf2c37..82a1f126b778 100644
> --- a/arch/mips/boot/dts/ingenic/ci20.dts
> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
> @@ -116,8 +116,8 @@
> pinctrl-0 = <_mmc1>;
>
> brcmf: wifi@1 {
> -/* reg = <4>;*/
> - compatible = "brcm,bcm4330-fmac";
> + reg = <1>;
> + compatible = "brcm,bcm4329-fmac";
> vcc-supply = <_power>;
> device-wakeup-gpios = < 9 GPIO_ACTIVE_HIGH>;
> shutdown-gpios = < 7 GPIO_ACTIVE_LOW>;
Do you have it working with a v5.8 kernel?
I don't see any activity to detect the module or load firmware.
Does it rely on some other patch?
BR and thanks,
Nikolaus
[PATCH v2] modpost: remove use of non-standard strsep() in HOSTCC code
strsep() is neither standard C nor POSIX and used outside
the kernel code here. Using it here requires that the
build host supports it out of the box which is e.g.
not true for a Darwin build host and using a cross-compiler.
This leads to:
scripts/mod/modpost.c:145:2: warning: implicit declaration of function 'strsep'
[-Wimplicit-function-declaration]
return strsep(stringp, "\n");
^
and a segfault when running MODPOST.
See also: https://stackoverflow.com/a/7219504
So let's replace this by strchr() instead of using strsep().
It does not hurt kernel size or speed since this code is run
on the build host.
Fixes: ac5100f5432967 ("modpost: add read_text_file() and get_line() helpers")
Co-developed-by: Masahiro Yamada
Signed-off-by: H. Nikolaus Schaller
---
scripts/mod/modpost.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 6aea65c657454..45f2ab2ec2d46 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -138,11 +138,19 @@ char *read_text_file(const char *filename)
char *get_line(char **stringp)
{
+ char *orig = *stringp, *next;
+
/* do not return the unwanted extra line at EOF */
- if (*stringp && **stringp == '\0')
+ if (!orig || *orig == '\0')
return NULL;
- return strsep(stringp, "\n");
+ next = strchr(orig, '\n');
+ if (next)
+ *next++ = '\0';
+
+ *stringp = next;
+
+ return orig;
}
/* A list of all modules we processed */
--
2.26.2
Re: [PATCH] modpost: remove use of non-standard strsep() in HOSTCC code
> Am 28.06.2020 um 09:52 schrieb Masahiro Yamada :
>
> On Sun, Jun 28, 2020 at 3:17 PM H. Nikolaus Schaller
> wrote:
>>
>> Hi,
>>
>>> Am 28.06.2020 um 07:51 schrieb Masahiro Yamada :
>>>
>>> On Thu, Jun 25, 2020 at 5:47 PM H. Nikolaus Schaller
>>> wrote:
>>>>
>>>> strsep() is neither standard C nor POSIX and used outside
>>>> the kernel code here. Using it here requires that the
>>>> build host supports it out of the box which is e.g.
>>>> not true for a Darwin build host and using a cross-compiler.
>>>> This leads to:
>>>>
>>>> scripts/mod/modpost.c:145:2: warning: implicit declaration of function
>>>> 'strsep' [-Wimplicit-function-declaration]
>>>> return strsep(stringp, "\n");
>>>> ^
>>>>
>>>> and a segfault when running MODPOST.
>>>>
>>>> See also: https://stackoverflow.com/a/7219504
>>>>
>>>> So let's add some lines of code separating the string at the
>>>> next newline character instead of using strsep(). It does not
>>>> hurt kernel size or speed since this code is run on the build host.
>>>>
>>>> Fixes: ac5100f5432967 ("modpost: add read_text_file() and get_line()
>>>> helpers")
>>>> Signed-off-by: H. Nikolaus Schaller
>>>> ---
>>>> scripts/mod/modpost.c | 7 ++-
>>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>>>> index 6aea65c65745..8fe63989c6e1 100644
>>>> --- a/scripts/mod/modpost.c
>>>> +++ b/scripts/mod/modpost.c
>>>> @@ -138,11 +138,16 @@ char *read_text_file(const char *filename)
>>>>
>>>> char *get_line(char **stringp)
>>>> {
>>>> + char *p;
>>>> /* do not return the unwanted extra line at EOF */
>>>> if (*stringp && **stringp == '\0')
>>>
>>> This check does not make sense anymore.
>>>
>>> Previously, get_line(NULL) returns NULL.
>>>
>>> With your patch, get_line(NULL) crashes
>>> due to NULL-pointer dereference.
>>
>> Well, that is original code.
>
>
> Sorry for confusion.
>
> I meant this:
>
> char *s = NULL;
> get_line();
>
>
> In the current code, get_line() returns NULL.
> As 'man strsep' says this:
> "If *stringp is NULL, the strsep() function returns NULL
> and does nothing else."
>
> With your patch, **stringp will cause
> NULL-pointer dereference.
Ah, now I see. strsep() has a special case that is not covered
by my patch.
On the other hand, get_line() is only called as get_line() and
pos = buf can not be NULL because that is checked before in read_dump().
This is why I did not observe a segfault.
But it is wise to make get_line() it more robust than needed. We do
never know who will copy this code fragment... And I am tempted to
handle the get_line(NULL) case as well.
>> I have only replaced the strsep() function.
>> But yes, it looks to be better in addition to
>> my patch.
>>
>>>
>>>
>>>
>>>> return NULL;
>>>>
>>>> - return strsep(stringp, "\n");
>>>> + p = *stringp;
>>>> + while (**stringp != '\n')
>>>> + (*stringp)++;
>>>
>>>
>>> Is this a safe conversion?
>>>
>>> If the input file does not contain '\n' at all,
>>> this while-loop continues running,
>>> and results in the segmentation fault
>>> due to buffer over-run.
>>
>> Ah, yes, you are right.
>>
>> We should use
>>
>> + while (**stringp && **stringp != '\n')
>>
>>>
>>>
>>>
>>>> + *(*stringp)++ = '\0';
>>>> + return p;
>>>> }
>>>
>>>
>>>
>>> How about this?
>>>
>>> char *get_line(char **stringp)
>>> {
>>> char *orig = *stringp;
>>
>> ^^^ this still segfaults with get_line(NULL)
>
>
> This is OK.
>
> get_line(NULL) should crash because we never expect
> the case ' stringp == NULL'.
>
> We need to care about the case ' *stringp == NULL'.
> In this case, get_line() should return NULL.
>
>
>
>
>>> char *next;
>>>
>>> /* do not return the unwanted extra line at EOF */
>>> if (!orig || *orig == '\0')
>>> return NULL;
>>>
>>> next = strchr(orig, '\n');
>>> if (next)
>>> *next++ = '\0';
>>>
>>> *stringp = next;
>>
>> Yes, this code is easier to understand than my while loop.
>> And strchr() is POSIX.
>>
>> So should I submit an updated patch or do you want to submit
>> it (with a suggested-by: H. Nikolaus Schaller )
>
> Please send a patch.
> (Co-developed-by if you want to give some credit to me)
Yes, I will do in the next days.
BR and thanks,
Nikolaus Schaller
Re: [PATCH] modpost: remove use of non-standard strsep() in HOSTCC code
Hi,
> Am 28.06.2020 um 07:51 schrieb Masahiro Yamada :
>
> On Thu, Jun 25, 2020 at 5:47 PM H. Nikolaus Schaller
> wrote:
>>
>> strsep() is neither standard C nor POSIX and used outside
>> the kernel code here. Using it here requires that the
>> build host supports it out of the box which is e.g.
>> not true for a Darwin build host and using a cross-compiler.
>> This leads to:
>>
>> scripts/mod/modpost.c:145:2: warning: implicit declaration of function
>> 'strsep' [-Wimplicit-function-declaration]
>> return strsep(stringp, "\n");
>> ^
>>
>> and a segfault when running MODPOST.
>>
>> See also: https://stackoverflow.com/a/7219504
>>
>> So let's add some lines of code separating the string at the
>> next newline character instead of using strsep(). It does not
>> hurt kernel size or speed since this code is run on the build host.
>>
>> Fixes: ac5100f5432967 ("modpost: add read_text_file() and get_line()
>> helpers")
>> Signed-off-by: H. Nikolaus Schaller
>> ---
>> scripts/mod/modpost.c | 7 ++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>> index 6aea65c65745..8fe63989c6e1 100644
>> --- a/scripts/mod/modpost.c
>> +++ b/scripts/mod/modpost.c
>> @@ -138,11 +138,16 @@ char *read_text_file(const char *filename)
>>
>> char *get_line(char **stringp)
>> {
>> + char *p;
>>/* do not return the unwanted extra line at EOF */
>>if (*stringp && **stringp == '\0')
>
> This check does not make sense anymore.
>
> Previously, get_line(NULL) returns NULL.
>
> With your patch, get_line(NULL) crashes
> due to NULL-pointer dereference.
Well, that is original code.
I have only replaced the strsep() function.
But yes, it looks to be better in addition to
my patch.
>
>
>
>>return NULL;
>>
>> - return strsep(stringp, "\n");
>> + p = *stringp;
>> + while (**stringp != '\n')
>> + (*stringp)++;
>
>
> Is this a safe conversion?
>
> If the input file does not contain '\n' at all,
> this while-loop continues running,
> and results in the segmentation fault
> due to buffer over-run.
Ah, yes, you are right.
We should use
+ while (**stringp && **stringp != '\n')
>
>
>
>> + *(*stringp)++ = '\0';
>> + return p;
>> }
>
>
>
> How about this?
>
> char *get_line(char **stringp)
> {
>char *orig = *stringp;
^^^ this still segfaults with get_line(NULL)
>char *next;
>
>/* do not return the unwanted extra line at EOF */
>if (!orig || *orig == '\0')
>return NULL;
>
>next = strchr(orig, '\n');
>if (next)
>*next++ = '\0';
>
>*stringp = next;
Yes, this code is easier to understand than my while loop.
And strchr() is POSIX.
So should I submit an updated patch or do you want to submit
it (with a suggested-by: H. Nikolaus Schaller )
BR and thanks,
Nikolaus Schaller
Re: [PATCH] Add default mux for pins that a free GPIO lines on the PocketBeagle
> Am 27.06.2020 um 15:55 schrieb Drew Fustini :
>
> These pins on the PocketBeagle P1 and P2 headers are connected to AM3358
> balls with gpio lines, and these pins are not used for any other
> peripherals by default. These GPIO lines are unclaimed and could be used
> by userspace program through the gpiod ABI. However, no driver will have
> set mux mode for the pins.
>
> This patch adds a "default" state in the am33xx_pinmux node and sets the
> pins to gpio output mux mode.
wouldn't it be more safe to set them to input mode?
BR,
Nikolaus Schaller
>
> Signed-off-by: Drew Fustini
> ---
> arch/arm/boot/dts/am335x-pocketbeagle.dts | 98 +++
> 1 file changed, 98 insertions(+)
>
> diff --git a/arch/arm/boot/dts/am335x-pocketbeagle.dts
> b/arch/arm/boot/dts/am335x-pocketbeagle.dts
> index f0b01b86..900dc6558701 100644
> --- a/arch/arm/boot/dts/am335x-pocketbeagle.dts
> +++ b/arch/arm/boot/dts/am335x-pocketbeagle.dts
> @@ -60,6 +60,104 @@ vmmcsd_fixed: fixedregulator0 {
> };
>
> _pinmux {
> +
> + pinctrl-names = "default";
> + pinctrl-0 = < _03_gpio _34_gpio _19_gpio _24_gpio
> + _33_gpio _22_gpio _18_gpio _10_gpio
> + _06_gpio _04_gpio _02_gpio _08_gpio
> + _17_gpio >;
> +
> + /* P2_03 (ZCZ ball T10) gpio0_23 0x824 */
> + P2_03_gpio: pinmux_P2_03_gpio {
> + pinctrl-single,pins = <
> + AM33XX_PADCONF(AM335X_PIN_GPMC_AD9, PIN_OUTPUT,
> MUX_MODE7)
> + >;
> + };
> +
> + /* P1_34 (ZCZ ball T11) gpio0_26 0x828 */
> + P1_34_gpio: pinmux_P1_34_gpio {
> + pinctrl-single,pins = <
> + AM33XX_PADCONF(AM335X_PIN_GPMC_AD10, PIN_OUTPUT,
> MUX_MODE7)
> + >;
> + };
> +
> + /* P2_19 (ZCZ ball U12) gpio0_27 0x82c */
> + P2_19_gpio: pinmux_P2_19_gpio {
> + pinctrl-single,pins = <
> + AM33XX_PADCONF(AM335X_PIN_GPMC_AD11, PIN_OUTPUT,
> MUX_MODE7)
> + >;
> + };
> +
> + /* P2_24 (ZCZ ball T12) gpio1_12 0x830 */
> + P2_24_gpio: pinmux_P2_24_gpio {
> + pinctrl-single,pins = <
> + AM33XX_PADCONF(AM335X_PIN_GPMC_AD12, PIN_OUTPUT,
> MUX_MODE7)
> + >;
> + };
> +
> + /* P2_33 (ZCZ ball R12) gpio1_13 0x834 */
> + P2_33_gpio: pinmux_P2_33_gpio {
> + pinctrl-single,pins = <
> + AM33XX_PADCONF(AM335X_PIN_GPMC_AD13, PIN_OUTPUT,
> MUX_MODE7)
> + >;
> + };
> +
> + /* P2_22 (ZCZ ball V13) gpio1_14 0x838 */
> + P2_22_gpio: pinmux_P2_22_gpio {
> + pinctrl-single,pins = <
> + AM33XX_PADCONF(AM335X_PIN_GPMC_AD14, PIN_OUTPUT,
> MUX_MODE7)
> + >;
> + };
> +
> + /* P2_18 (ZCZ ball U13) gpio1_15 0x83c */
> + P2_18_gpio: pinmux_P2_18_gpio {
> + pinctrl-single,pins = <
> + AM33XX_PADCONF(AM335X_PIN_GPMC_AD15, PIN_OUTPUT,
> MUX_MODE7)
> + >;
> + };
> +
> + /* P2_10 (ZCZ ball R14) gpio1_20 0x850 */
> + P2_10_gpio: pinmux_P2_10_gpio {
> + pinctrl-single,pins = <
> + AM33XX_PADCONF(AM335X_PIN_GPMC_A4, PIN_OUTPUT,
> MUX_MODE7)
> + >;
> + };
> +
> + /* P2_06 (ZCZ ball U16) gpio1_25 0x864 */
> + P2_06_gpio: pinmux_P2_06_gpio {
> + pinctrl-single,pins = <
> + AM33XX_PADCONF(AM335X_PIN_GPMC_A9, PIN_OUTPUT,
> MUX_MODE7)
> + >;
> + };
> +
> + /* P2_04 (ZCZ ball T16) gpio1_26 0x868 */
> + P2_04_gpio: pinmux_P2_04_gpio {
> + pinctrl-single,pins = <
> + AM33XX_PADCONF(AM335X_PIN_GPMC_A10, PIN_OUTPUT,
> MUX_MODE7)
> + >;
> + };
> +
> + /* P2_02 (ZCZ ball V17) gpio1_27 0x86c */
> + P2_02_gpio: pinmux_P2_02_gpio {
> + pinctrl-single,pins = <
> + AM33XX_PADCONF(AM335X_PIN_GPMC_A11, PIN_OUTPUT,
> MUX_MODE7)
> + >;
> + };
> +
> + /* P2_08 (ZCZ ball U18) gpio1_28 0x878 */
> + P2_08_gpio: pinmux_P2_08_gpio {
> + pinctrl-single,pins = <
> + AM33XX_PADCONF(AM335X_PIN_GPMC_BEN1, PIN_OUTPUT,
> MUX_MODE7)
> + >;
> + };
> +
> + /* P2_17 (ZCZ ball V12) gpio2_1 0x88c */
> + P2_17_gpio: pinmux_P2_17_gpio {
> + pinctrl-single,pins = <
> + AM33XX_PADCONF(AM335X_PIN_GPMC_CLK, PIN_OUTPUT,
> MUX_MODE7)
> + >;
> + };
> +
> i2c2_pins: pinmux-i2c2-pins {
> pinctrl-single,pins = <
> AM33XX_PADCONF(AM335X_PIN_UART1_RTSN, PIN_INPUT_PULLUP,
> MUX_MODE3) /* (D17) uart1_rtsn.I2C2_SCL */
> --
> 2.25.1
>
[PATCH] modpost: remove use of non-standard strsep() in HOSTCC code
strsep() is neither standard C nor POSIX and used outside
the kernel code here. Using it here requires that the
build host supports it out of the box which is e.g.
not true for a Darwin build host and using a cross-compiler.
This leads to:
scripts/mod/modpost.c:145:2: warning: implicit declaration of function 'strsep'
[-Wimplicit-function-declaration]
return strsep(stringp, "\n");
^
and a segfault when running MODPOST.
See also: https://stackoverflow.com/a/7219504
So let's add some lines of code separating the string at the
next newline character instead of using strsep(). It does not
hurt kernel size or speed since this code is run on the build host.
Fixes: ac5100f5432967 ("modpost: add read_text_file() and get_line() helpers")
Signed-off-by: H. Nikolaus Schaller
---
scripts/mod/modpost.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 6aea65c65745..8fe63989c6e1 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -138,11 +138,16 @@ char *read_text_file(const char *filename)
char *get_line(char **stringp)
{
+ char *p;
/* do not return the unwanted extra line at EOF */
if (*stringp && **stringp == '\0')
return NULL;
- return strsep(stringp, "\n");
+ p = *stringp;
+ while (**stringp != '\n')
+ (*stringp)++;
+ *(*stringp)++ = '\0';
+ return p;
}
/* A list of all modules we processed */
--
2.26.2
[PATCH 2/4] w1: omap-hdq: fix return value to be -1 if there is a timeout
omap_w1_read_byte() should return -1 (or 0xff) in case of
error (e.g. missing battery).
The code accidentially overwrites the variable ret and not val,
which is returned. So it will return the initial value 0 instead
of -1.
Fixes: 27d13da8782a ("w1: omap-hdq: Simplify driver with PM runtime
autosuspend")
Cc: sta...@vger.kernel.org # v5.6+
Signed-off-by: H. Nikolaus Schaller
---
drivers/w1/masters/omap_hdq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/w1/masters/omap_hdq.c b/drivers/w1/masters/omap_hdq.c
index d363e2a89fdfc4..9f9ec108b189e2 100644
--- a/drivers/w1/masters/omap_hdq.c
+++ b/drivers/w1/masters/omap_hdq.c
@@ -464,7 +464,7 @@ static u8 omap_w1_read_byte(void *_hdq)
ret = hdq_read_byte(hdq_data, );
if (ret)
- ret = -1;
+ val = -1;
pm_runtime_mark_last_busy(hdq_data->dev);
pm_runtime_put_autosuspend(hdq_data->dev);
--
2.26.2
[PATCH 4/4] w1: omap-hdq: print dev_err if irq flags are not cleared
If irq flags are not cleared for certain operations we print an error message. Since this should never occur in normal operation, this patch is an optional safety-net and debugging tool. Signed-off-by: H. Nikolaus Schaller --- drivers/w1/masters/omap_hdq.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/w1/masters/omap_hdq.c b/drivers/w1/masters/omap_hdq.c index a6484700f3b388..bf2ec59c1f9ddc 100644 --- a/drivers/w1/masters/omap_hdq.c +++ b/drivers/w1/masters/omap_hdq.c @@ -146,6 +146,10 @@ static int hdq_write_byte(struct hdq_data *hdq_data, u8 val, u8 *status) goto rtn; } + if (hdq_data->hdq_irqstatus) + dev_err(hdq_data->dev, "TX irqstatus not cleared (%02x)\n", + hdq_data->hdq_irqstatus); + *status = 0; hdq_reg_out(hdq_data, OMAP_HDQ_TX_DATA, val); @@ -243,6 +247,10 @@ static int omap_hdq_break(struct hdq_data *hdq_data) goto rtn; } + if (hdq_data->hdq_irqstatus) + dev_err(hdq_data->dev, "break irqstatus not cleared (%02x)\n", + hdq_data->hdq_irqstatus); + /* set the INIT and GO bit */ hdq_reg_merge(hdq_data, OMAP_HDQ_CTRL_STATUS, OMAP_HDQ_CTRL_STATUS_INITIALIZATION | OMAP_HDQ_CTRL_STATUS_GO, -- 2.26.2
[PATCH 1/4] w1: omap-hdq: cleanup to add missing newline for some dev_dbg
Otherwise it will corrupt the console log during debugging.
Fixes: 7b5362a603a1 ("w1: omap_hdq: Fix some error/debug handling.")
Cc: sta...@vger.kernel.org
Signed-off-by: H. Nikolaus Schaller
---
drivers/w1/masters/omap_hdq.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/w1/masters/omap_hdq.c b/drivers/w1/masters/omap_hdq.c
index aa09f85277767a..d363e2a89fdfc4 100644
--- a/drivers/w1/masters/omap_hdq.c
+++ b/drivers/w1/masters/omap_hdq.c
@@ -155,7 +155,7 @@ static int hdq_write_byte(struct hdq_data *hdq_data, u8
val, u8 *status)
/* check irqstatus */
if (!(*status & OMAP_HDQ_INT_STATUS_TXCOMPLETE)) {
dev_dbg(hdq_data->dev, "timeout waiting for"
- " TXCOMPLETE/RXCOMPLETE, %x", *status);
+ " TXCOMPLETE/RXCOMPLETE, %x\n", *status);
ret = -ETIMEDOUT;
goto out;
}
@@ -166,7 +166,7 @@ static int hdq_write_byte(struct hdq_data *hdq_data, u8
val, u8 *status)
OMAP_HDQ_FLAG_CLEAR, _status);
if (ret) {
dev_dbg(hdq_data->dev, "timeout waiting GO bit"
- " return to zero, %x", tmp_status);
+ " return to zero, %x\n", tmp_status);
}
out:
@@ -183,7 +183,7 @@ static irqreturn_t hdq_isr(int irq, void *_hdq)
spin_lock_irqsave(_data->hdq_spinlock, irqflags);
hdq_data->hdq_irqstatus = hdq_reg_in(hdq_data, OMAP_HDQ_INT_STATUS);
spin_unlock_irqrestore(_data->hdq_spinlock, irqflags);
- dev_dbg(hdq_data->dev, "hdq_isr: %x", hdq_data->hdq_irqstatus);
+ dev_dbg(hdq_data->dev, "hdq_isr: %x\n", hdq_data->hdq_irqstatus);
if (hdq_data->hdq_irqstatus &
(OMAP_HDQ_INT_STATUS_TXCOMPLETE | OMAP_HDQ_INT_STATUS_RXCOMPLETE
@@ -248,7 +248,7 @@ static int omap_hdq_break(struct hdq_data *hdq_data)
tmp_status = hdq_data->hdq_irqstatus;
/* check irqstatus */
if (!(tmp_status & OMAP_HDQ_INT_STATUS_TIMEOUT)) {
- dev_dbg(hdq_data->dev, "timeout waiting for TIMEOUT, %x",
+ dev_dbg(hdq_data->dev, "timeout waiting for TIMEOUT, %x\n",
tmp_status);
ret = -ETIMEDOUT;
goto out;
@@ -275,7 +275,7 @@ static int omap_hdq_break(struct hdq_data *hdq_data)
_status);
if (ret)
dev_dbg(hdq_data->dev, "timeout waiting INIT bits"
- " return to zero, %x", tmp_status);
+ " return to zero, %x\n", tmp_status);
out:
hdq_reset_irqstatus(hdq_data);
--
2.26.2
[PATCH 3/4] w1: omap-hdq: fix interrupt handling which did show spurious timeouts
Since
commit 27d13da8782a ("w1: omap-hdq: Simplify driver with PM runtime
autosuspend")
was applied,
I did see timeouts and wrong values when reading a bq27000 connected
to hdq of the omap3. This occurred mainly after boot but remained and
only sometimes settled down after several reads.
root@letux:~# time cat /sys/class/power_supply/bq27000-battery/uevent
POWER_SUPPLY_NAME=bq27000-battery
POWER_SUPPLY_STATUS=Discharging
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_VOLTAGE_NOW=0
POWER_SUPPLY_CURRENT_NOW=0
POWER_SUPPLY_CAPACITY=0
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_TEMP=-2731
POWER_SUPPLY_TIME_TO_EMPTY_NOW=0
POWER_SUPPLY_TIME_TO_EMPTY_AVG=0
POWER_SUPPLY_TIME_TO_FULL_NOW=0
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CHARGE_FULL=0
POWER_SUPPLY_CHARGE_NOW=0
POWER_SUPPLY_CHARGE_FULL_DESIGN=0
POWER_SUPPLY_CYCLE_COUNT=0
POWER_SUPPLY_ENERGY_NOW=0
POWER_SUPPLY_POWER_AVG=0
POWER_SUPPLY_HEALTH=Good
POWER_SUPPLY_MANUFACTURER=Texas Instruments
real 0m15.761s
user 0m0.001s
sys 0m0.025s
root@letux:~#
Sometimes the effect did disappear after accessing
the device multiple times, speed went up and results
became correct.
All this indicates that some interrupts from the hdq
controller are lost by the driver.
Enabling debugging revealed that there were spurious tx
and rx timeouts, i.e. the driver does not always recognise
interrupts. The main problem is that rx and tx interrupts
share a single variable which was sometimes reset to
0 wiping out other interrupts. And it was overwritten
by a second interrupt, independent of whether the
previous interrupt was already processed or not.
This patch improves interrupt handling to avoid such
races and loss of interrupt flags.
The ideas are:
* only the hdq_isr() sets bits in hdq_status
* it does not reset any bits
* it does wake_up() if any interrupt is pending
* bits are only reset by the read/write/break functions
if they were waited for
* this makes sure that no interrupts can be lost
* rx/tx/timeout bits are completely decoupled from each
other (and not reset all after waiting for any of them)
* which bits to reset is now specified by a new parameter
to hdq_reset_irqstatus()
* hdq_reset_irqstatus() also returns the state before
resetting so that we can encapsulate the spinlock
* this should now handle the case that the write and read
are both already finished quickly before the hdq_write_byte()
ends.
* Or that two interrupts occur in succession before
they are processed by the driver.
Old code may have reset all status bits making the next
hdq_read_byte() timeout.
* the spinlock now always protects changing of bits in function
hdq_reset_irqstatus() which could become a read-write-modify
problem if the interrupt handler tries to read-modify-write
exactly at the same moment
* we add mutex protection also for hdq_write_byte() just to
be safe to not to disturb a hdq_read_byte() triggered by
some other thread/process.
This patch was tested on a GTA04 and results in no
boot problems any more. And first read after boot is now ok:
root@letux:~# time cat /sys/class/power_supply/bq27000-battery/uevent
POWER_SUPPLY_NAME=bq27000-battery
POWER_SUPPLY_STATUS=Discharging
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_VOLTAGE_NOW=397
POWER_SUPPLY_CURRENT_NOW=354144
POWER_SUPPLY_CAPACITY=82
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_TEMP=266
POWER_SUPPLY_TIME_TO_EMPTY_NOW=7680
POWER_SUPPLY_TIME_TO_EMPTY_AVG=7380
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CHARGE_FULL=934856
POWER_SUPPLY_CHARGE_NOW=763976
POWER_SUPPLY_CHARGE_FULL_DESIGN=1233792
POWER_SUPPLY_CYCLE_COUNT=82
POWER_SUPPLY_ENERGY_NOW=2852840
POWER_SUPPLY_POWER_AVG=1392840
POWER_SUPPLY_HEALTH=Good
POWER_SUPPLY_MANUFACTURER=Texas Instruments
real0m0.233s
user0m0.000s
sys 0m0.025s
root@letux:~#
It was also tested with dev_dbg enabled and more
printk that all activities behave correctly, especially
hdq_write_byte(), hdq_read_byte(), omap_hdq_break().
Not tested is omap_w1_triplet().
Fixes: 27d13da8782a ("w1: omap-hdq: Simplify driver with PM runtime
autosuspend")
Cc: sta...@vger.kernel.org # v5.6+
Signed-off-by: H. Nikolaus Schaller
---
drivers/w1/masters/omap_hdq.c | 62 ---
1 file changed, 42 insertions(+), 20 deletions(-)
diff --git a/drivers/w1/masters/omap_hdq.c b/drivers/w1/masters/omap_hdq.c
index 9f9ec108b189e2..a6484700f3b388 100644
--- a/drivers/w1/masters/omap_hdq.c
+++ b/drivers/w1/masters/omap_hdq.c
@@ -54,10 +54,10 @@ MODULE_PARM_DESC(w1_id, "1-wire id for the slave detection
in HDQ mode");
struct hdq_data {
struct device *dev;
void __iomem*hdq_base;
- /* lock status update */
+ /* lock read/write/break operations */
struct mutex hdq_mutex;
+ /* interrupt status and a lock for it */
u8 hdq_irqstatus;
- /* device lock */
spinlock_t hdq_spinlock;
/* m
[PATCH 0/4] w1: omap: fix some regressions/bugs (some were introduced in v5.6 but some are older)
This series fixes: * some dev_dbg are missing an explicit \n * wrong return value if battery is removed and no hdq response * problems with resetting interrupt flags too early leading to timeouts and wrong values * print error if interrupt flags get mixed up H. Nikolaus Schaller (4): w1: omap-hdq: cleanup to add missing newline for some dev_dbg w1: omap-hdq: fix return value to be -1 if there is a timeout w1: omap-hdq: fix interrupt handling which did show spurious timeouts w1: omap-hdq: print dev_err if irq flags are not cleared drivers/w1/masters/omap_hdq.c | 82 --- 1 file changed, 56 insertions(+), 26 deletions(-) -- 2.26.2
Re: [PATCH v7 01/12] dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
> Am 05.05.2020 um 17:53 schrieb Rob Herring : > > On Fri, Apr 24, 2020 at 10:34:04PM +0200, H. Nikolaus Schaller wrote: >> The Imagination PVR/SGX GPU is part of several SoC from >> multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo, >> Allwinner A83 and others. >> >> With this binding, we describe how the SGX processor is >> interfaced to the SoC (registers and interrupt). >> >> The interface also consists of clocks, reset, power but >> information from data sheets is vague and some SoC integrators >> (TI) deciced to use a PRCM wrapper (ti,sysc) which does >> all clock, reset and power-management through registers >> outside of the sgx register block. >> >> Therefore all these properties are optional. >> >> Tested by make dt_binding_check >> >> Signed-off-by: H. Nikolaus Schaller >> --- >> .../devicetree/bindings/gpu/img,pvrsgx.yaml | 150 ++ >> 1 file changed, 150 insertions(+) >> +oneOf: >> + - description: SGX530-121 based SoC >> +items: >> + - enum: >> +- ti,omap3-sgx530-121 # BeagleBoard A/B/C, OpenPandora 600MHz >> and similar > > Should be indented 2 more here and elsewhere where you have a list > under a list. added for patch v8 series. BR and thanks, Nikolaus
Re: [PATCH v7 01/12] dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
Hi Tony, > Am 03.05.2020 um 17:01 schrieb Tony Lindgren : > > * Paul Cercueil [200503 14:19]: >> You have a new SoC with a SGX, and you only need to enable one clock to get >> it to work. So you create a devicetree node which receives only one clock. >> >> Turns out, that the bootloader was enabling the other 3 clocks, and since >> the last release, it doesn't anymore. You're left with having to support a >> broken devicetree. >> >> That's the kind of problem that can be easily avoided by enforcing the >> number of clocks that have to be provided. > > The number of clocks depends on how it's wired for the SoC. > > On omaps, there's are no controls for additinoal SGX clocks. Sure some > of the clocks may be routed to multple places internally by the wrapper > module. But we have no control over that. > > If we wanted to specify just the "fck" clock on omaps, then we can > do it with something like this: > > allOf: > - if: >properites: > compatible: >enum: > - "ti,omap4-sgx544-112" > - "ti,omap5-sgx544-116" > - "ti,dra7-sgx544-116" >then: > properties: >clocks: > minItems: 1 > maxItems: 1 > >clock-names: > const: fck > >required: > - clocks > - clock-names will add to v8 of this series as a separate patch on top of the general one. This should make it easier to have a focussed discussion and revert/bisect if something goes wrong. BR and thanks, Nikolaus
Re: [PATCH v7 01/12] dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
Hi Paul & Paul, > Am 03.05.2020 um 18:41 schrieb H. Nikolaus Schaller : > > Hi Paul and Paul, > >> Am 03.05.2020 um 16:18 schrieb Paul Cercueil : >> >> >> >> Le dim. 3 mai 2020 à 15:31, H. Nikolaus Schaller a >> écrit : >>> Hi Paul, >>>> Am 03.05.2020 um 14:52 schrieb Paul Cercueil : >>>>>> It's possible to forbid the presence of the 'clocks' property on some >>>>>> implementations, and require it on others. >>>>> To be precise we have to specify the exact number of clocks (between 0 >>>>> and 4) for every architecture. >>>>> This also contradicts my dream to get rid of the architecture specific >>>>> components in the long run. My dream (because I can't tell how it can be >>>>> done) is that we can one day develop something which just needs >>>>> compatible = img,530 or imp,540 or img,544. Then we can't make the number >>>>> clocks depend on the implementation any more. >>>> As we said before, the number of clocks is a property of the GPU and *not* >>>> its integration into the SoC. >>> Well, it is a not very well documented property of the GPU. We have no data >>> sheet of the standalone GPU. Only several SoC data sheets which give some >>> indications. >> >> Maybe we can nicely ask them? > > There is some (old) answer here: > > https://github.com/MIPS/CI20_linux/blob/ci20-v3.18/arch/mips/boot/dts/jz4780.dtsi#L63 > >> I expect Paul Burton to have some contacts at ImgTec. Asking for a doc would >> be too much, but maybe they can help a bit with the DT bindings. > > Good idea! It is definitively worth to try. Therefore I have moved him from > CC: to To: Do we already have an idea if we can get into contact and get help from ImgTec for this topic or if we have to live with what we have? BR and thanks, Nikolaus
Bug with omap3-isp - 30 seconds delay for probe success
resend from correct mail address --- Hi Tony, I am observing an issue with omap3-isp for a while. It seems to have started with v5.6 but I have preferred to invest some time into analysis of the problem instead of trying a bisect. The problem is that there is a [ 32.483703] WARNING: CPU: 0 PID: 2052 at drivers/base/dd.c:270 driver_deferred_probe_check_state+0x44/0x5c [ 32.498809] omap3isp 480bc000.isp: deferred probe timeout, ignoring dependency from the driver, just after exactly 30 seconds. This is when driver_deferred_probe_timeout had timed out (chaning driver_deferred_probe_timeout makes it take more or less time). So something is requested for by the omap3-isp driver which never becomes available. Some analysis shows that the omap3-isp is the only device calling of_iommu_xlate() with a NULL opp table which ends up in driver_deferred_probe_check_state() to return -EPROBE_DEFER until 30 seconds have passed. Well, it seems to be resonable that there are no ops returned by iommu_ops_from_fwnode() since there is no firmware for the ISP. But there should be no timeout. This of_iommu_xlate() is called from of_iommu_configure() in the loop to handle all "iommus" references. There is one for omap34xx and omap36xx.dtsi and the mmu_isp is defined in omap3.dtsi. They refer to compatible = "ti,omap2-iommu"; and ti,hwmods = "mmu_isp"; Are there any ideas what in the iommu or hwmods or firmware loading for the mmu_isp may have changed recently? Anyways the omap3-isp seems to be initialized after this 30 seconds timeout and responds to media-ctl. A more complete log attached. BR, Nikolaus [ 32.478759] [ cut here ] [ 32.483703] WARNING: CPU: 0 PID: 2052 at drivers/base/dd.c:270 driver_deferred_probe_check_state+0x44/0x5c [ 32.498809] omap3isp 480bc000.isp: deferred probe timeout, ignoring dependency [ 32.498840] Modules linked in: omapdrm libertas_sdio libertas cfg80211 panel_tpo_td028ttec1 snd_soc_simple_card snd_soc_simple_card_utils snd_soc_omap_twl4030 simple_bridge wwan_on_off pvrsrvkm_omap3630_sgx530_125 snd_soc_gtm601 pwm_omap_dmtimer omap_aes_driver crypto_engine omap_crypto omap_sham omap3_isp videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_common bq27xxx_battery_hdq bq27xxx_battery omap_hdq omap2430 bmp280_spi bmp280_i2c itg3200 bmp280 at24 tsc2007 leds_tca6507 bma180 phy_twl4030_usb musb_hdrc twl4030_pwrbutton hci_uart snd_soc_twl4030 twl4030_vibra btbcm ov9655 twl4030_madc v4l2_fwnode twl4030_charger videodev hmc5843_i2c bluetooth hmc5843_core gnss_sirf industrialio_triggered_buffer mc kfifo_buf ecdh_generic snd_soc_si47xx ecc gnss snd_soc_omap_mcbsp snd_soc_ti_sdma ehci_omap omapdss omapdss_base drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops cec display_connector generic_adc_battery drm industrialio drm_panel_orientation_quirks input_polldev [ 32.509704] pwm_bl ip_tables x_tables ipv6 nf_defrag_ipv6 autofs4 [ 32.612792] CPU: 0 PID: 2052 Comm: kworker/0:5 Not tainted 5.7.0-rc4-letux+ #2570 [ 32.620758] Hardware name: Generic OMAP36xx (Flattened Device Tree) [ 32.627471] Workqueue: events deferred_probe_work_func [ 32.632995] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 32.641235] [] (show_stack) from [] (dump_stack+0x88/0xa8) [ 32.648925] [] (dump_stack) from [] (__warn+0xc8/0xf4) [ 32.656249] [] (__warn) from [] (warn_slowpath_fmt+0x70/0x9c) [ 32.664215] [] (warn_slowpath_fmt) from [] (driver_deferred_probe_check_state+0x44/0x5c) [ 32.674652] [] (driver_deferred_probe_check_state) from [] (of_iommu_configure+0x98/0x1b4) [ 32.685302] [] (of_iommu_configure) from [] (of_dma_configure+0x1d8/0x234) [ 32.694458] [] (of_dma_configure) from [] (really_probe+0x104/0x324) [ 32.703063] [] (really_probe) from [] (driver_probe_device+0x10c/0x154) [ 32.711944] [] (driver_probe_device) from [] (bus_for_each_drv+0x90/0xb8) [ 32.721008] [] (bus_for_each_drv) from [] (__device_attach+0x90/0x120) [ 32.729797] [] (__device_attach) from [] (bus_probe_device+0x28/0x80) [ 32.738494] [] (bus_probe_device) from [] (deferred_probe_work_func+0x5c/0x80) [ 32.748016] [] (deferred_probe_work_func) from [] (process_one_work+0x1e4/0x394) [ 32.757720] [] (process_one_work) from [] (process_scheduled_works+0x28/0x30) [ 32.767150] [] (process_scheduled_works) from [] (worker_thread+0x210/0x2d8) [ 32.776489] [] (worker_thread) from [] (kthread+0x138/0x148) [ 32.784362] [] (kthread) from [] (ret_from_fork+0x14/0x2c) [ 32.792022] Exception stack(0xda649fb0 to 0xda649ff8) [ 32.797393] 9fa0: [ 32.806091] 9fc0: [ 32.814788] 9fe0: 0013 [ 32.842346] omap3isp 480bc000.isp: supply vdd-csiphy1 not found, using dummy regulator [ 32.855133] omap3isp 480bc000.isp: supply
Re: [PATCHv3] w1: omap-hdq: Simplify driver with PM runtime autosuspend
Hi Tony, > Am 29.04.2020 um 23:38 schrieb Tony Lindgren : > > * H. Nikolaus Schaller [200429 21:35]: >> I have reworked the way the spinlocks, setting and resetting >> of the hdq_irqstatus bits are done and now it works right from >> start of boot. Without any timeouts or delays. >> >> I am not exactly sure what went wrong, but it seems as if >> the read is already done when the write interrupt status >> bit is processed. Then, the old logic did wipe out both >> bits by hdq_reset_irqstatus() and the read code did timeout >> because it did not notice that the data had already been >> available. This may depend on other system activities so >> that it can explain why other tests didn't reveal it. >> >> omap_hdq_runtime_resume() and omap_hdq_runtime_suspend() >> also behave fine. >> >> Before I can post something I need to clean up my hacks >> and add similar fixes to omap_hdq_break() and omap_w1_triplet() >> where I hope that I don't break those... > > OK good to hear you were able to figure out what is > going on here. I have found another small bug and a dev_dbg format weakness and now it seems to work well even if I remove or reinsert the battery while read operations are ongoing. Still I need more time to fix up the patch(es). BR and thanks, Nikolaus
[PATCH] drm: ingenic-drm: add MODULE_DEVICE_TABLE
so that the driver can load by matching the device tree
if compiled as module.
Cc: sta...@vger.kernel.org # v5.3+
Fixes: 90b86fcc47b4 ("DRM: Add KMS driver for the Ingenic JZ47xx SoCs")
Signed-off-by: H. Nikolaus Schaller
---
drivers/gpu/drm/ingenic/ingenic-drm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c
b/drivers/gpu/drm/ingenic/ingenic-drm.c
index 9dfe7cb530e11..1754c05470690 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -843,6 +843,7 @@ static const struct of_device_id ingenic_drm_of_match[] = {
{ .compatible = "ingenic,jz4770-lcd", .data = _soc_info },
{ /* sentinel */ },
};
+MODULE_DEVICE_TABLE(of, ingenic_drm_of_match);
static struct platform_driver ingenic_drm_driver = {
.driver = {
--
2.26.2
Re: [PATCH v7 01/12] dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
Hi Paul and Paul, > Am 03.05.2020 um 16:18 schrieb Paul Cercueil : > > > > Le dim. 3 mai 2020 à 15:31, H. Nikolaus Schaller a écrit > : >> Hi Paul, >>> Am 03.05.2020 um 14:52 schrieb Paul Cercueil : >>>>> It's possible to forbid the presence of the 'clocks' property on some >>>>> implementations, and require it on others. >>>> To be precise we have to specify the exact number of clocks (between 0 and >>>> 4) for every architecture. >>>> This also contradicts my dream to get rid of the architecture specific >>>> components in the long run. My dream (because I can't tell how it can be >>>> done) is that we can one day develop something which just needs compatible >>>> = img,530 or imp,540 or img,544. Then we can't make the number clocks >>>> depend on the implementation any more. >>> As we said before, the number of clocks is a property of the GPU and *not* >>> its integration into the SoC. >> Well, it is a not very well documented property of the GPU. We have no data >> sheet of the standalone GPU. Only several SoC data sheets which give some >> indications. > > Maybe we can nicely ask them? There is some (old) answer here: https://github.com/MIPS/CI20_linux/blob/ci20-v3.18/arch/mips/boot/dts/jz4780.dtsi#L63 > I expect Paul Burton to have some contacts at ImgTec. Asking for a doc would > be too much, but maybe they can help a bit with the DT bindings. Good idea! It is definitively worth to try. Therefore I have moved him from CC: to To: > >> It appears as if some sgx5xx versions have 3 clocks and some have 4. So you >> are right, the number of clocks depends on the sgx5xx version and that could >> be made dependent in the bindings (if necessary). >>> So you would *not* have a number of clocks between 0 and 4. You get either >>> 0, or 4, depending on whether or not you have a wrapper. >> I think this is contradicting your previous sentence. If the number of >> clocks is a property of the GPU and not the integration it must also not >> depend on whether there is a wrapper. I.e. it must be a constant for any >> type of integration. > > Well, I expected all SGX versions to have 4 clocks. > > If some SGX versions have 3 clocks, and others have 4 clocks, it's still OK > as long as the number of clocks is enforced, so that all implementations of a > given SGX core will have to use the same number of clocks. > >> The really correct variant would be to always make the SoC integration >> (wrapper) a separate subsystem (because it is never part of the SGX core but >> some interface bus) and clock provider and connect it explicitly to the >> clock inputs. > > About the wrapper... I don't really know how it's done there. But you could > very well pass all clocks unconditionally to the SGX node, even if it's > inside a wrapper. > The wrapper itself probably needs only one clock, the one that allows it to > access its registers. > >> To be clear: I am not at all against describing the clocks. I just doubt >> that the time we invest into discussing on this level of detail and adding >> conditional clock requirements is worth the result. IMHO the bindings and >> .dts do not become better by describing them in more detail than just >> "optional". It just takes our time from contributing to other subsystems. > > You have a new SoC with a SGX, and you only need to enable one clock to get > it to work. So you create a devicetree node which receives only one clock. > > Turns out, that the bootloader was enabling the other 3 clocks, Does it? I haven't seen such boot loaders. Usually they bring up only the core and e.g. mmc to be able to boot. > and since the last release, it doesn't anymore. You're left with having to > support a broken devicetree. > > That's the kind of problem that can be easily avoided by enforcing the number > of clocks that have to be provided. >>>>> See how it's done for instance on >>>>> Documentation/devicetree/bindings/serial/samsung_uart.yaml. >>>> Yes I know the design pattern, but I wonder if such a move makes the whole >>>> thing even less maintainable. >>>> Assume we have finished DTS for some SoC. Then these DTS have been tested >>>> on real hardware and are working. Clocks are there where needed and >>>> missing where not. We may now forbid or not forbid them for some >>>> implementations in the bindings.yaml but the result of dtbs_check won't >>>> change! Because they are tested and working and the bindings.yaml has been >>>
Re: [PATCH v7 01/12] dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
Hi Paul, > Am 03.05.2020 um 14:52 schrieb Paul Cercueil : > >>> It's possible to forbid the presence of the 'clocks' property on some >>> implementations, and require it on others. >> To be precise we have to specify the exact number of clocks (between 0 and >> 4) for every architecture. >> This also contradicts my dream to get rid of the architecture specific >> components in the long run. My dream (because I can't tell how it can be >> done) is that we can one day develop something which just needs compatible = >> img,530 or imp,540 or img,544. Then we can't make the number clocks depend >> on the implementation any more. > > As we said before, the number of clocks is a property of the GPU and *not* > its integration into the SoC. Well, it is a not very well documented property of the GPU. We have no data sheet of the standalone GPU. Only several SoC data sheets which give some indications. It appears as if some sgx5xx versions have 3 clocks and some have 4. So you are right, the number of clocks depends on the sgx5xx version and that could be made dependent in the bindings (if necessary). > > So you would *not* have a number of clocks between 0 and 4. You get either 0, > or 4, depending on whether or not you have a wrapper. I think this is contradicting your previous sentence. If the number of clocks is a property of the GPU and not the integration it must also not depend on whether there is a wrapper. I.e. it must be a constant for any type of integration. The really correct variant would be to always make the SoC integration (wrapper) a separate subsystem (because it is never part of the SGX core but some interface bus) and clock provider and connect it explicitly to the clock inputs. To be clear: I am not at all against describing the clocks. I just doubt that the time we invest into discussing on this level of detail and adding conditional clock requirements is worth the result. IMHO the bindings and .dts do not become better by describing them in more detail than just "optional". It just takes our time from contributing to other subsystems. > > >>> See how it's done for instance on >>> Documentation/devicetree/bindings/serial/samsung_uart.yaml. >> Yes I know the design pattern, but I wonder if such a move makes the whole >> thing even less maintainable. >> Assume we have finished DTS for some SoC. Then these DTS have been tested on >> real hardware and are working. Clocks are there where needed and missing >> where not. We may now forbid or not forbid them for some implementations in >> the bindings.yaml but the result of dtbs_check won't change! Because they >> are tested and working and the bindings.yaml has been adapted to the result. >> So we have just duplicated something for no practical benefit. >> Next, assume there is coming support for more and more new SoC. Then, >> developers not only have to figure out which clocks they need in the DTS but >> they also have to add a patch to the implementation specific part of the >> bindings.yaml to clearly define exactly the same what they already have >> written into their .dts (the clocks are either there for the of_node or they >> are not). So again the rules are for no benefit, since a new SoC is >> introduced exactly once. And tested if it works. And if it is there, it will >> stay as it is. It is just work for maintainers to review that patch as well. > > If you add support for a new SoC, you'd still need to modify the binding to > add the compatible string. So the argument of "more work" is moot. Agreed, I forgot this aspect. Nevertheless, it is easier to review a new compatible string than a new clock number rule (question: how do you practically review this? By looking if it does match the DTS?). We have to add the compatible string as long as we need to have the SoC name in the compatible string (which as said is my dream to get rid of in far future). If we could get rid of it, there won't be a change any more. By just taking "img,sgx544" into a new SoC. The change would be moved into SoC specific wrappers. In such an ideal world, we would explicitly describe the wrappers as separate DT nodes. Even if they have no explicit driver (e.g. by some simple-pm-bus). PRCM,bus, Processor <<>> Wrapper <<->> SGX ti,... ti,sysc img,sgx530 img,...simple-busimg,sgx540 samsung,...... img,sgx544 other, other,gpu-wrapper img,... But this IMHO correct proposal was already rejected. So at the moment we are circling around several proposals because none can fulfill all requirements. Therefore my attempt to solve the gordian knot is to make clocks generally optional. This keeps the bindings simple but not generally wrong. And since the DTS are not only tested against bindings.yaml but on real hardware, the omission to enforce a specific number of clocks doesn't harm anyone. As said it is
Re: [PATCH v7 01/12] dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
Hi Paul, > Am 26.04.2020 um 15:11 schrieb Paul Cercueil : > > Hi Nikolaus, > > Le ven. 24 avril 2020 à 22:34, H. Nikolaus Schaller a > écrit : >> The Imagination PVR/SGX GPU is part of several SoC from >> multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo, >> Allwinner A83 and others. >> With this binding, we describe how the SGX processor is >> interfaced to the SoC (registers and interrupt). >> The interface also consists of clocks, reset, power but >> information from data sheets is vague and some SoC integrators >> (TI) deciced to use a PRCM wrapper (ti,sysc) which does >> all clock, reset and power-management through registers >> outside of the sgx register block. >> Therefore all these properties are optional. >> Tested by make dt_binding_check >> Signed-off-by: H. Nikolaus Schaller >> --- >> .../devicetree/bindings/gpu/img,pvrsgx.yaml | 150 ++ >> 1 file changed, 150 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml >> diff --git a/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml >> b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml >> new file mode 100644 >> index ..33a9c4c6e784 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml >> @@ -0,0 +1,150 @@ >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/gpu/img,pvrsgx.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Imagination PVR/SGX GPU >> + >> +maintainers: >> + - H. Nikolaus Schaller >> + >> +description: |+ >> + This binding describes the Imagination SGX5 series of 3D accelerators >> which >> + are found in several different SoC like TI OMAP, Sitara, Ingenic JZ4780, >> + Allwinner A83, and Intel Poulsbo and CedarView and more. >> + >> + For an extensive list see: >> https://en.wikipedia.org/wiki/PowerVR#Implementations >> + >> + The SGX node is usually a child node of some DT node belonging to the SoC >> + which handles clocks, reset and general address space mapping of the SGX >> + register area. If not, an optional clock can be specified here. >> + >> +properties: >> + $nodename: >> +pattern: '^gpu@[a-f0-9]+$' >> + compatible: >> +oneOf: >> + - description: SGX530-121 based SoC >> +items: >> + - enum: >> +- ti,omap3-sgx530-121 # BeagleBoard A/B/C, OpenPandora 600MHz >> and similar >> + - const: img,sgx530-121 >> + - const: img,sgx530 >> + >> + - description: SGX530-125 based SoC >> +items: >> + - enum: >> +- ti,am3352-sgx530-125 # BeagleBone Black >> +- ti,am3517-sgx530-125 >> +- ti,am4-sgx530-125 >> +- ti,omap3-sgx530-125 # BeagleBoard XM, GTA04, OpenPandora 1GHz >> and similar >> +- ti,ti81xx-sgx530-125 >> + - const: ti,omap3-sgx530-125 >> + - const: img,sgx530-125 >> + - const: img,sgx530 >> + >> + - description: SGX535-116 based SoC >> +items: >> + - const: intel,poulsbo-gma500-sgx535 # Atom Z5xx >> + - const: img,sgx535-116 >> + - const: img,sgx535 >> + >> + - description: SGX540-116 based SoC >> +items: >> + - const: intel,medfield-gma-sgx540 # Atom Z24xx >> + - const: img,sgx540-116 >> + - const: img,sgx540 >> + >> + - description: SGX540-120 based SoC >> +items: >> + - enum: >> +- samsung,s5pv210-sgx540-120 >> +- ti,omap4-sgx540-120 # Pandaboard, Pandaboard ES and similar >> + - const: img,sgx540-120 >> + - const: img,sgx540 >> + >> + - description: SGX540-130 based SoC >> +items: >> + - enum: >> +- ingenic,jz4780-sgx540-130 # CI20 >> + - const: img,sgx540-130 >> + - const: img,sgx540 >> + >> + - description: SGX544-112 based SoC >> +items: >> + - const: ti,omap4470-sgx544-112 >> + - const: img,sgx544-112 >> + - const: img,sgx544 >> + >> + - description: SGX544-115 based SoC >> +items: >> + - enum: >> +- allwinner,sun8i-a31-sgx544-115 >> +- allwinner,sun8i-a31s-sgx
Re: [PATCHv3] w1: omap-hdq: Simplify driver with PM runtime autosuspend
Hi, > Am 25.04.2020 um 12:37 schrieb H. Nikolaus Schaller : > > >> Am 25.04.2020 um 12:29 schrieb H. Nikolaus Schaller : >> >> H >> The things start to get "fixed" when the hdq_isr >> jumps to 6 indicating >> >> OMAP_HDQ_INT_STATUS_RXCOMPLETE | OMAP_HDQ_INT_STATUS_TXCOMPLETE >> >> So I am getting more inclined to believe that it is >> not a power management issue but some piggybacked >> change to how interrupts are handled. >> Especially hdq_reset_irqstatus. >> >> So I will add a printk to hdq_reset_irqstatus >> to see what value it had before being reset. > > I now did check the log during boot and there is the > reverse situation. Initially it works but suddenly > hdq_isr becomes 6 and then trouble starts. > > So the key problem is that both, the RX and the TX > interrupts may be set and then, the code resets > everything to 0 and looses either one. > > I wonder if that is an issue by two processes reading > hdq in parallel. > > Another question is how independent command-writes + result-reads > are properly serialized and locked so that they don't overlap? I have reworked the way the spinlocks, setting and resetting of the hdq_irqstatus bits are done and now it works right from start of boot. Without any timeouts or delays. I am not exactly sure what went wrong, but it seems as if the read is already done when the write interrupt status bit is processed. Then, the old logic did wipe out both bits by hdq_reset_irqstatus() and the read code did timeout because it did not notice that the data had already been available. This may depend on other system activities so that it can explain why other tests didn't reveal it. omap_hdq_runtime_resume() and omap_hdq_runtime_suspend() also behave fine. Before I can post something I need to clean up my hacks and add similar fixes to omap_hdq_break() and omap_w1_triplet() where I hope that I don't break those... BR and thanks, Nikolaus
Re: [PATCH v7 06/12] ARM: DTS: omap4: add sgx gpu child node
Hi Paul,
> Am 26.04.2020 um 14:50 schrieb Paul Cercueil :
>
> Hi Nikolaus,
>
> Le ven. 24 avril 2020 à 22:34, H. Nikolaus Schaller a
> écrit :
>> Add SGX GPU node with interrupt. Tested on PandaBoard ES.
>> Since omap4420/30/60 and omap4470 come with different SGX variants
>> we need to introduce a new omap4470.dtsi. If an omap4470 board
>> does not want to use SGX it is no problem to still include
>> omap4460.dtsi.
>> Signed-off-by: H. Nikolaus Schaller
>> ---
>> arch/arm/boot/dts/omap4.dtsi | 11 ++-
>> arch/arm/boot/dts/omap4470.dts | 15 +++
>> 2 files changed, 21 insertions(+), 5 deletions(-)
>> create mode 100644 arch/arm/boot/dts/omap4470.dts
>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>> index 763bdea8c829..15ff3d7146af 100644
>> --- a/arch/arm/boot/dts/omap4.dtsi
>> +++ b/arch/arm/boot/dts/omap4.dtsi
>> @@ -389,7 +389,7 @@ abb_iva: regulator-abb-iva {
>> status = "disabled";
>> };
>> -target-module@5600 {
>> +sgx_module: target-module@5600 {
>> compatible = "ti,sysc-omap4", "ti,sysc";
>> reg = <0x5600fe00 0x4>,
>><0x5600fe10 0x4>;
>> @@ -408,10 +408,11 @@ target-module@5600 {
>> #size-cells = <1>;
>> ranges = <0 0x5600 0x200>;
>> -/*
>> - * Closed source PowerVR driver, no child device
>> - * binding or driver in mainline
>> - */
>> +gpu: gpu@0 {
>> +compatible = "ti,omap4-sgx540-120",
>> "img,sgx540-120", "img,sgx540";
>> +reg = <0x0 0x200>; /* 32MB */
>> +interrupts = ;
>> +};
>> };
>> /*
>> diff --git a/arch/arm/boot/dts/omap4470.dts b/arch/arm/boot/dts/omap4470.dts
>> new file mode 100644
>> index ..f29c581300bf
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/omap4470.dts
^^^ there is also a missing "i" in the file name
>> @@ -0,0 +1,15 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Device Tree Source for OMAP4470 SoC
>> + *
>> + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
>> + *
>> + * This file is licensed under the terms of the GNU General Public License
>> + * version 2. This program is licensed "as is" without any warranty of any
>> + * kind, whether express or implied.
>> + */
>> +#include "omap4460.dtsi"
>> +
>> + {
>
> Does this even compile?
Good question.
So far there is no well known eval board in mainline that #includes this .dtsi
(because it is new) and therefore it passes any compile tests.
DTC arch/arm/boot/dts/omap4470-test.dtb - due to target missing
Error: arch/arm/boot/dts/omap4470.dtsi:13.1-5 Label or path sgx not found
I have now added a dummy board (not to be mainlined) for my own compile test...
>
> The node's handle is named sgx_module, not sgx.
Indeed. A fix is queued for v8.
BR and thanks,
Nikolaus
Re: [PATCH 1/2] configs: ARM: omap2plus: Enable OMAP3_THERMAL
> Am 23.10.2019 um 00:19 schrieb Tony Lindgren : > > * Adam Ford [191022 19:01]: >> On Tue, Oct 22, 2019 at 11:22 AM Tony Lindgren wrote: >>> >>> Hi, >>> >>> * Adam Ford [191007 15:06]: The some in the OMAP3 family have a bandgap thermal sensor, but omap2plus has it disabled. This patch enables the OMAP3_THERMAL by default like the rest of the OMAP family. >>> >>> Looks like this breaks off mode during idle for omap3, and that's >>> probably why it never got enabled. The difference in power >>> consumption during idle is about 7mW vs 32mW for the SoC as >>> measured from torpedo shunt for main_battery_som. >>> >>> I think the right fix might be simply to add handling for >>> CPU_CLUSTER_PM_ENTER to the related thermal driver to disable >>> it during idle like we have for gpio-omap.c for example. >> >> I am not sure I know where to start on fixing that issue. Would you >> entertain enabling the driver if we set the device tree to 'disabled' >> by default? This way if people want to to use it, it can be enabled >> on a per-device option. Once the power stuff gets resolved, we might >> be able to enable it by default. For people who are planning on using >> the DM3730 @ 1GHz in high temp environments, I am not sure they'll >> care about low power. > > They should both work fine together though. They are not mutually > exclusive features. > >> I'll try to look into it when I have time, but I was hoping a >> compromise might be a reasonable work-around. > > It should be hopefully a trivial fix.. I have not looked at the > driver code though. If I am taken right, it is the drivers/thermal/ti-soc-thermal/ti-*.c which is a common driver for omap3, omap4, omap5. They only differ in the thermal data and which registers and bits are used to access the ADC. So is this problem with off mode also known for omap4 and omap5? BR, Nikolaus
Re: [PATCH 1/7] dt-bindings: gpu: pvrsgx: add initial bindings
> Am 22.10.2019 um 17:36 schrieb Tony Lindgren : > > * H. Nikolaus Schaller [191022 15:12]: >> Hm. How should that work? Some SoC have the sgx544 as single >> core and others as dual core. This imho does not fit into >> the "img,sgx544-$revision" scheme which could be matched to. > > Well don't you have then just two separate child nodes, > one for each core with their own register range? Doesn't look so. AFAIK the architecture of SGX is that there is a single scheduler which is accessed by the register range and it internally has control over multiple cores. > > That is assuming they're really separate instances.. No. There is some internal magic in the driver. It just needs to know that there is one or two nodes. Currently this is handled by some #define option when calling the inner Makefile. My idea was to replace the #ifdef by checking for the img,cores property. > >> But maybe we do it the same as with the timer for the moment, >> i.e. keep it in some unpublished patch set. > > Yeah makes sense to me until things get sorted out. > >> At the moment I have more problems understanding how the yaml >> thing works. I understand and fully support the overall goal, >> but it is quite difficult to get a start here. And there do not >> seem to be tools or scripts to help converting from old style >> text format (even if not perfect, this would be helpful) and >> I have no yaml editor that helps keeping the indentation >> correct. So this slows down a v2 a little bit. > > Sounds handy to me :) > > Regards, > > Tony
Re: [PATCH 3/9] DTS: ARM: pandora-common: define wl1251 as child node of mmc3
> Am 21.10.2019 um 19:13 schrieb Tony Lindgren :
>
> * H. Nikolaus Schaller [191018 20:28]:
>> Since v4.7 the dma initialization requires that there is a
>> device tree property for "rx" and "tx" channels which is
>> not provided by the pdata-quirks initialization.
>>
>> By conversion of the mmc3 setup to device tree this will
>> finally allows to remove the OpenPandora wlan specific omap3
>> data-quirks.
>>
>> Fixes: 81eef6ca9201 ("mmc: omap_hsmmc: Use dma_request_chan() for requesting
>> DMA channel")
>
> Here you have the subject line the wrong way around,
> please update it to start with "ARM: dts: ...".
Ok.
Re: [PATCH v2 07/11] omap: remove old hsmmc.[ch] and in Makefile
> Am 21.10.2019 um 16:30 schrieb Tony Lindgren : > > * H. Nikolaus Schaller [191019 18:43]: >> --- a/arch/arm/mach-omap2/Makefile >> +++ b/arch/arm/mach-omap2/Makefile >> @@ -216,7 +216,6 @@ obj-$(CONFIG_MACH_NOKIA_N8X0)+= board-n8x0.o >> >> # Platform specific device init code >> >> -omap-hsmmc-$(CONFIG_MMC_OMAP_HS):= hsmmc.o >> obj-y+= $(omap-hsmmc-m) >> $(omap-hsmmc-y) > > The related obj-y line can go now too, right? Yes, I think so. It is a construction that I have never seen before :) Therefore I did not recognize that it is related. > And looks like common.h also has struct omap2_hsmmc_info > so maybe check by grepping for hsmmc_info to see it's gone. Yes, it is just a forward-declaration of the struct name with no user anywhere. Scheduled for v3. BTW: should this series go through your tree since it is an omap machine? BR and thanks, Nikolaus
Re: [Letux-kernel] [PATCH 5/5] rtc: rtc-rc5t583: add ricoh rc5t619 RTC driver
Just a meta-comment...
> Am 21.10.2019 um 12:15 schrieb Alexandre Belloni
> :
>
> Hi,
>
> The subject line is weird, how is it related to rc5t583?
>
> On 21/10/2019 07:41:04+0200, Andreas Kemnade wrote:
>> config RTC_DRV_S35390A
>> tristate "Seiko Instruments S-35390A"
>> select BITREVERSE
>> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
>> index 6b09c21dc1b6..1d0673fd0954 100644
>> --- a/drivers/rtc/Makefile
>> +++ b/drivers/rtc/Makefile
>> @@ -136,6 +136,7 @@ obj-$(CONFIG_RTC_DRV_PXA)+= rtc-pxa.o
>> obj-$(CONFIG_RTC_DRV_R7301) += rtc-r7301.o
>> obj-$(CONFIG_RTC_DRV_R9701) += rtc-r9701.o
>> obj-$(CONFIG_RTC_DRV_RC5T583)+= rtc-rc5t583.o
>> +obj-$(CONFIG_RTC_DRV_RC5T619) += rtc-rc5t619.o
>> obj-$(CONFIG_RTC_DRV_RK808) += rtc-rk808.o
>> obj-$(CONFIG_RTC_DRV_RP5C01) += rtc-rp5c01.o
>> obj-$(CONFIG_RTC_DRV_RS5C313)+= rtc-rs5c313.o
>> diff --git a/drivers/rtc/rtc-rc5t619.c b/drivers/rtc/rtc-rc5t619.c
>> new file mode 100644
>> index ..311788ff0723
>> --- /dev/null
>> +++ b/drivers/rtc/rtc-rc5t619.c
>> @@ -0,0 +1,476 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * drivers/rtc/rtc-ricoh619.c
>> + *
>> + * Real time clock driver for RICOH R5T619 power management chip.
>> + *
>> + * Copyright (C) 2019 Andreas Kemnade
>> + *
>> + * Based on code
>> + * Copyright (C) 2012-2014 RICOH COMPANY,LTD
>> + *
>> + * Based on code
>> + * Copyright (C) 2011 NVIDIA Corporation
>
> Based on is not useful.
Yes, it is difficult to track what the real contribution was
and what is left over.
On the other hand it is IMHO fair to attribute those who have
spent time to save ours.
What would be a better way for attribution?
>
>> + */
>> +
>> +/* #define debug1 */
>> +/* #define verbose_debug1 */
>> +
>
> No dead code please.
>
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +struct rc5t619_rtc {
>> +int irq;
>> +struct rtc_device *rtc;
>> +struct rn5t618 *rn5t618;
>> +};
>> +
>> +#define CTRL1_ALARM_ENABLED 0x40
>> +#define CTRL1_24HR 0x20
>> +#define CTRL1_PERIODIC_MASK 0xf
>> +
>> +#define CTRL2_PON 0x10
>> +#define CTRL2_ALARM_STATUS 0x80
>> +#define CTRL2_CTFG 0x4
>> +#define CTRL2_CTC 0x1
>> +
>> +static int rc5t619_rtc_periodic_disable(struct device *dev)
>> +{
>> +struct rc5t619_rtc *rtc = dev_get_drvdata(dev);
>> +int err;
>> +
>> +/* disable function */
>> +err = regmap_update_bits(rtc->rn5t618->regmap,
>> +RN5T618_RTC_CTRL1, CTRL1_PERIODIC_MASK, 0);
>> +if (err < 0)
>> +return err;
>> +
>> +/* clear alarm flag and CTFG */
>> +err = regmap_update_bits(rtc->rn5t618->regmap, RN5T618_RTC_CTRL2,
>> +CTRL2_ALARM_STATUS | CTRL2_CTFG | CTRL2_CTC, 0);
>> +if (err < 0)
>> +return err;
>> +
>> +return 0;
>> +}
>> +
>> +static int rc5t619_rtc_clk_adjust(struct device *dev, uint8_t clk)
>> +{
>> +struct rc5t619_rtc *rtc = dev_get_drvdata(dev);
>> +
>> +return regmap_write(rtc->rn5t618->regmap, RN5T618_RTC_ADJUST, clk);
>
> Is it useful to have a function for a single regmap_write?
I'd say yes. It is wrapping all regmap accesses in getter/setter functions
whose name describes what it is setting. And it may do type conversion.
IMHO this makes code better readable and maintainable.
And a good compiler may even decide to inline this.
>
> Also what is that adjusting? If this is adding/removing clock cycles,
> you need to use .set_offset and .read_offset.
>
>> +}
>> +
>> +static int rc5t619_rtc_pon_get_clr(struct device *dev, uint8_t *pon_f)
>> +{
>> +struct rc5t619_rtc *rtc = dev_get_drvdata(dev);
>> +int err;
>> +unsigned int reg_data;
>> +
>> +err = regmap_read(rtc->rn5t618->regmap, RN5T618_RTC_CTRL2, _data);
>> +if (err < 0)
>> +return err;
>> +
>> +if (reg_data & CTRL2_PON) {
>> +*pon_f = 1;
>> +/* clear VDET PON */
>> +reg_data &= ~(CTRL2_PON | CTRL2_CTC | 0x4a);/* 0101-1011 */
>> +reg_data |= 0x20; /* 0010- */
>
> Is it possible to have more defines for those magic values?
>
>> +err = regmap_write(rtc->rn5t618->regmap, RN5T618_RTC_CTRL2,
>> +reg_data);
>> +} else {
>> +*pon_f = 0;
>> +}
>> +
>> +return err;
>> +}
>> +
>> +/* 0-12hour, 1-24hour */
>> +static int rc5t619_rtc_24hour_mode_set(struct device *dev, int mode)
>> +{
>> +struct rc5t619_rtc *rtc = dev_get_drvdata(dev);
>> +
>> +return regmap_update_bits(rtc->rn5t618->regmap, RN5T618_RTC_CTRL1,
>> +CTRL1_24HR, mode ? CTRL1_24HR : 0);
>
> Is it useful to have a function for a single regmap_update_bits?
Same as above. I can immediately understand
r = rc5t619_rtc_24hour_mode_set(dev, MODE_SOMETHING);
[PATCH v2 09/11] mmc: core: fix wl1251 sdio quirks
wl1251 and wl1271 have different vendor id and device id.
So we need to handle both with sdio quirks.
Fixes: 884f38607897 ("mmc: core: move some sdio IDs out of quirks file")
Signed-off-by: H. Nikolaus Schaller
Cc: # 4.11.0
---
drivers/mmc/core/quirks.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h
index 2d2d9ea8be4f..3dba15bccce2 100644
--- a/drivers/mmc/core/quirks.h
+++ b/drivers/mmc/core/quirks.h
@@ -119,7 +119,14 @@ static const struct mmc_fixup mmc_ext_csd_fixups[] = {
END_FIXUP
};
+
static const struct mmc_fixup sdio_fixup_methods[] = {
+ SDIO_FIXUP(SDIO_VENDOR_ID_TI_WL1251, SDIO_DEVICE_ID_TI_WL1251,
+ add_quirk, MMC_QUIRK_NONSTD_FUNC_IF),
+
+ SDIO_FIXUP(SDIO_VENDOR_ID_TI_WL1251, SDIO_DEVICE_ID_TI_WL1251,
+ add_quirk, MMC_QUIRK_DISABLE_CD),
+
SDIO_FIXUP(SDIO_VENDOR_ID_TI, SDIO_DEVICE_ID_TI_WL1271,
add_quirk, MMC_QUIRK_NONSTD_FUNC_IF),
--
2.19.1
[PATCH v2 08/11] mmc: sdio: fix wl1251 vendor id
v4.11-rc1 did introduce a patch series that rearranged the
sdio quirks into a header file. Unfortunately this did forget
to handle SDIO_VENDOR_ID_TI differently between wl1251 and
wl1271 with the result that although the wl1251 was found on
the sdio bus, the firmware did not load any more and there was
no interface registration.
This patch defines separate constants to be used by sdio quirks
and drivers.
Fixes: 884f38607897 ("mmc: core: move some sdio IDs out of quirks file")
Signed-off-by: H. Nikolaus Schaller
Cc: # 4.11.0
---
include/linux/mmc/sdio_ids.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/mmc/sdio_ids.h b/include/linux/mmc/sdio_ids.h
index d1a5d5df02f5..08b25c02b5a1 100644
--- a/include/linux/mmc/sdio_ids.h
+++ b/include/linux/mmc/sdio_ids.h
@@ -71,6 +71,8 @@
#define SDIO_VENDOR_ID_TI 0x0097
#define SDIO_DEVICE_ID_TI_WL1271 0x4076
+#define SDIO_VENDOR_ID_TI_WL1251 0x104c
+#define SDIO_DEVICE_ID_TI_WL1251 0x9066
#define SDIO_VENDOR_ID_STE 0x0020
#define SDIO_DEVICE_ID_STE_CW1200 0x2280
--
2.19.1
[PATCH v2 04/11] mmc: host: omap_hsmmc: add code for special init of wl1251 to get rid of pandora_wl1251_init_card
Pandora_wl1251_init_card was used to do special pdata based
setup of the sdio mmc interface. This does no longer work with
v4.7 and later. A fix requires a device tree based mmc3 setup.
Therefore we move the special setup to omap_hsmmc.c instead
of calling some pdata supplied init_card function.
The new code checks for a DT child node compatible to wl1251
so it will not affect other MMC3 use cases.
Fixes: 81eef6ca9201 ("mmc: omap_hsmmc: Use dma_request_chan() for requesting
DMA channel")
Signed-off-by: H. Nikolaus Schaller
Cc: # 4.7.0
---
drivers/mmc/host/omap_hsmmc.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 952fa4063ff8..03ba80bcf319 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1512,6 +1512,27 @@ static void omap_hsmmc_init_card(struct mmc_host *mmc,
struct mmc_card *card)
if (mmc_pdata(host)->init_card)
mmc_pdata(host)->init_card(card);
+ else if (card->type == MMC_TYPE_SDIO || card->type ==
MMC_TYPE_SD_COMBO) {
+ struct device_node *np = mmc_dev(mmc)->of_node;
+
+ np = of_get_compatible_child(np, "ti,wl1251");
+ if (np) {
+ /*
+* We have TI wl1251 attached to MMC3. Pass this
information to
+* SDIO core because it can't be probed by normal
methods.
+*/
+
+ dev_info(host->dev, "found wl1251\n");
+ card->quirks |= MMC_QUIRK_NONSTD_SDIO;
+ card->cccr.wide_bus = 1;
+ card->cis.vendor = 0x104c;
+ card->cis.device = 0x9066;
+ card->cis.blksize = 512;
+ card->cis.max_dtr = 2400;
+ card->ocr = 0x80;
+ of_node_put(np);
+ }
+ }
}
static void omap_hsmmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
--
2.19.1
[PATCH v2 01/11] Documentation: dt: wireless: update wl1251 for sdio
The standard method for sdio devices connected to
an sdio interface is to define them as a child node
like we can see with wlcore.
Signed-off-by: H. Nikolaus Schaller
Acked-by: Kalle Valo
---
.../bindings/net/wireless/ti,wl1251.txt | 26 +++
1 file changed, 26 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
index bb2fcde6f7ff..88612ff29f2d 100644
--- a/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
@@ -35,3 +35,29 @@ Examples:
ti,power-gpio = < 23 GPIO_ACTIVE_HIGH>; /* 87 */
};
};
+
+ {
+ vmmc-supply = <_en>;
+
+ bus-width = <4>;
+ non-removable;
+ ti,non-removable;
+ cap-power-off-card;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ wlan: wl1251@1 {
+ compatible = "ti,wl1251";
+
+ reg = <1>;
+
+ interrupt-parent = <>;
+ interrupts = <21 IRQ_TYPE_LEVEL_HIGH>; /* GPIO_21 */
+
+ ti,wl1251-has-eeprom;
+ };
+};
--
2.19.1
