* Re: [PATCH] Input: xpad - fix GPF in xpad_probe
From: Dan Carpenter @ 2023-04-22 19:56 UTC (permalink / raw)
To: Dan Carpenter
Cc: Dongliang Mu, Vicki Pfau, kernel-janitors, Dmitry Torokhov,
Pavel Rojtberg, Nate Yocom, Mattijs Korpershoek, John Butler,
Matthias Benkmann, Christopher Crockett, Santosh De Massari,
hust-os-kernel-patches, syzbot+a3f758b8d8cb7e49afec,
Pierre-Loup A. Griffais, linux-input, linux-kernel
In-Reply-To: <d49bb5a2-ed78-4b16-b7ea-b9c7748ca0e4@kili.mountain>
On Sat, Apr 22, 2023 at 10:48:32PM +0300, Dan Carpenter wrote:
> The warnings from this are quite promising.
Well... The results are over 90% false positives. But there are still
probably 15 bugs in there. The printk warning trick would probably
work.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] Input: xpad - fix GPF in xpad_probe
From: Dan Carpenter @ 2023-04-22 19:48 UTC (permalink / raw)
To: Dan Carpenter
Cc: Dongliang Mu, Vicki Pfau, kernel-janitors, Dmitry Torokhov,
Pavel Rojtberg, Nate Yocom, Mattijs Korpershoek, John Butler,
Matthias Benkmann, Christopher Crockett, Santosh De Massari,
hust-os-kernel-patches, syzbot+a3f758b8d8cb7e49afec,
Pierre-Loup A. Griffais, linux-input, linux-kernel
In-Reply-To: <3ccc85bd-0541-4ffb-a207-dfc533a2c0aa@kili.mountain>
[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]
The warnings from this are quite promising.
When you're writing a check, you start with a simple idea and then try
it and then filter out the common false positives.
The first 10 warnings are from loops like:
p = NULL;
for (i = 0; i < limit; i++) {
if (i == 0)
p = non_null();
else
*p = something();
}
Smatch doesn't handle loops correctly. (I know how to fix this but I've
never gotten around to it because it would make Smatch slow)...
So instead of that maybe I would do a hack to silence this type of
warning. Not sure what...
drivers/usb/gadget/udc/amd5536udc_pci.c:61 udc_pci_remove() warn: pointer dereferenced without being set '&udc->gadget'
This one is interesting. Seems like a real bug.
drivers/mtd/ubi/block.c:391 ubiblock_create() warn: pointer dereferenced without being set 'dev->gd'
This one too. So maybe we could make this a separate warning where
NULL dereferences happen on error paths. Or maybe when they happen in
printks.
So there are ways to take this first draft and massage it and get
fewer false positives, by filtering false positives or taking things
which work and creating new checks instead.
Anyway, results attached.
regards,
dan carpenter
[-- Attachment #2: err-list --]
[-- Type: text/plain, Size: 26958 bytes --]
drivers/usb/core/devio.c:989 parse_usbdevfs_streams() warn: pointer dereferenced without being set 'intf'
drivers/usb/gadget/udc/amd5536udc_pci.c:61 udc_pci_remove() warn: pointer dereferenced without being set '&udc->gadget'
drivers/usb/gadget/udc/snps_udc_core.c:807 udc_create_dma_chain() warn: pointer dereferenced without being set 'last'
drivers/usb/gadget/udc/mv_udc_core.c:419 req_to_dtd() warn: pointer dereferenced without being set 'last_dtd'
drivers/usb/cdns3/cdnsp-mem.c:348 cdnsp_alloc_segments_for_ring() warn: pointer dereferenced without being set '*first->next'
drivers/usb/cdns3/cdnsp-mem.c:450 cdnsp_ring_expansion() warn: pointer dereferenced without being set 'first->next'
drivers/mtd/ubi/block.c:391 ubiblock_create() warn: pointer dereferenced without being set 'dev->gd'
drivers/mtd/nftlmount.c:135 find_boot_record() warn: pointer dereferenced without being set 'nftl->ReplUnitTable'
drivers/mtd/mtdswap.c:328 mtdswap_read_markers() warn: pointer dereferenced without being set 'd->mtd'
drivers/mtd/mtdswap.c:388 mtdswap_write_marker() warn: pointer dereferenced without being set 'd->mtd'
drivers/mtd/mtdswap.c:464 mtdswap_scan_eblks() warn: pointer dereferenced without being set 'd->mtd'
drivers/mtd/spi-nor/core.c:1701 spi_nor_init_erase_cmd_list() warn: pointer dereferenced without being set 'cmd'
drivers/iommu/dma-iommu.c:1274 iommu_dma_map_sg() warn: pointer dereferenced without being set 'prev'
drivers/firmware/arm_scmi/notify.c:1709 scmi_notification_exit() warn: pointer dereferenced without being set 'ni->notify_wq'
drivers/infiniband/ulp/rtrs/rtrs-clt.c:2866 rtrs_clt_open() warn: pointer dereferenced without being set 'clt->dev.p'
drivers/staging/rtl8712/rtl871x_mlme.c:367 update_scanned_network() warn: pointer dereferenced without being set 'pnetwork'
./include/linux/list.h:75 __list_add() warn: pointer dereferenced without being set '&(prev->next)'
drivers/staging/nvec/nvec_power.c:97 nvec_power_notifier() warn: pointer dereferenced without being set 'nvec_psy'
drivers/staging/nvec/nvec_power.c:166 nvec_power_bat_notifier() warn: pointer dereferenced without being set 'nvec_bat_psy'
drivers/staging/nvec/nvec_power.c:430 nvec_power_remove() warn: pointer dereferenced without being set 'nvec_bat_psy'
drivers/staging/nvec/nvec_power.c:427 nvec_power_remove() warn: pointer dereferenced without being set 'nvec_psy'
drivers/staging/emxx_udc/emxx_udc.c:2695 nbu2ss_ep_set_halt() warn: pointer dereferenced without being set 'ep->udc'
drivers/staging/emxx_udc/emxx_udc.c:2743 nbu2ss_ep_fifo_status() warn: pointer dereferenced without being set 'ep->udc'
drivers/staging/emxx_udc/emxx_udc.c:2785 nbu2ss_ep_fifo_flush() warn: pointer dereferenced without being set 'ep->udc'
drivers/clk/meson/g12a.c:5263 meson_g12a_dvfs_setup_common() warn: pointer dereferenced without being set 'g12a_cpu_clk_dyn.hw.core'
drivers/clk/meson/g12a.c:5253 meson_g12a_dvfs_setup_common() warn: pointer dereferenced without being set 'g12a_cpu_clk_postmux0.hw.core'
drivers/clk/meson/g12a.c:5341 meson_g12b_dvfs_setup() warn: pointer dereferenced without being set 'g12a_sys_pll.hw.core'
drivers/clk/meson/g12a.c:5290 meson_g12b_dvfs_setup() warn: pointer dereferenced without being set 'g12b_cpu_clk.hw.core'
drivers/clk/meson/g12a.c:5332 meson_g12b_dvfs_setup() warn: pointer dereferenced without being set 'g12b_cpub_clk.hw.core'
drivers/clk/meson/g12a.c:5323 meson_g12b_dvfs_setup() warn: pointer dereferenced without being set 'g12b_cpub_clk_dyn.hw.core'
drivers/clk/meson/g12a.c:5313 meson_g12b_dvfs_setup() warn: pointer dereferenced without being set 'g12b_cpub_clk_postmux0.hw.core'
drivers/clk/meson/g12a.c:5300 meson_g12b_dvfs_setup() warn: pointer dereferenced without being set 'g12b_sys1_pll.hw.core'
drivers/clk/meson/g12a.c:5364 meson_g12a_dvfs_setup() warn: pointer dereferenced without being set 'g12a_cpu_clk.hw.core'
drivers/clk/meson/g12a.c:5373 meson_g12a_dvfs_setup() warn: pointer dereferenced without being set 'g12a_sys_pll.hw.core'
drivers/block/drbd/drbd_req.c:624 __req_mod() warn: pointer dereferenced without being set 'peer_device'
drivers/block/drbd/drbd_nl.c:1049 drbd_determine_dev_size() warn: pointer dereferenced without being set 'device->bitmap'
drivers/tty/serial/8250/8250_aspeed_vuart.c:345 aspeed_vuart_handle_irq() warn: pointer dereferenced without being set '&port->state->port'
drivers/hsi/clients/hsi_char.c:186 hsc_msg_alloc() warn: pointer dereferenced without being set 'msg->sgt.sgl'
drivers/isdn/hardware/mISDN/mISDNinfineon.c:1112 inf_probe() warn: pointer dereferenced without being set 'card->ipac.isac.dch.dev.D.st'
drivers/isdn/hardware/mISDN/mISDNinfineon.c:1112 inf_probe() warn: pointer dereferenced without being set 'card->ipac.isac.dch.dev.dev.p'
drivers/target/iscsi/iscsi_target.c:790 iscsi_target_cleanup_module() warn: pointer dereferenced without being set 'iscsit_global->discovery_tpg'
drivers/target/iscsi/iscsi_target.c:3650 iscsit_thread_get_cpumask() warn: pointer dereferenced without being set 'iscsit_global->allowed_cpumask'
drivers/target/iscsi/iscsi_target.c:3677 iscsit_thread_reschedule() warn: pointer dereferenced without being set 'iscsit_global->allowed_cpumask'
drivers/target/iscsi/iscsi_target_configfs.c:1169 lio_target_wwn_cpus_allowed_list_store() warn: pointer dereferenced without being set 'iscsit_global->allowed_cpumask'
drivers/gpu/drm/nouveau/nouveau_chan.c:500 nouveau_channel_new() warn: pointer dereferenced without being set '*pchan->userd'
drivers/gpu/drm/nouveau/nvkm/subdev/mmu/uvmm.c:389 nvkm_uvmm_new() warn: pointer dereferenced without being set 'uvmm->vmm'
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_packet_manager.c:280 pm_send_set_resources() warn: pointer dereferenced without being set 'pm->priv_queue->rptr_kernel'
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_packet_manager.c:293 pm_send_set_resources() warn: pointer dereferenced without being set 'pm->priv_queue->wptr_kernel'
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_chardev.c:1167 kfd_ioctl_alloc_memory_of_gpu() warn: pointer dereferenced without being set 'mem->bo'
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_chardev.c:1585 kfd_ioctl_import_dmabuf() warn: pointer dereferenced without being set 'mem->bo'
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_chardev.c:2309 criu_restore_memory_of_gpu() warn: pointer dereferenced without being set '*kgd_mem->bo'
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:994 subvp_vblank_schedulable() warn: pointer dereferenced without being set 'subvp_pipe'
drivers/acpi/pfr_telemetry.c:173 set_pfrt_log_level() warn: pointer dereferenced without being set 'out_obj->package.elements'
drivers/acpi/pfr_telemetry.c:201 get_pfrt_log_level() warn: pointer dereferenced without being set 'out_obj->package.elements'
drivers/dma-buf/dma-resv.c:577 dma_resv_get_fences() warn: pointer dereferenced without being set '*fences'
drivers/hid/hid-multitouch.c:1107 mt_process_slot() warn: pointer dereferenced without being set 'slot->a'
drivers/md/dm-cache-target.c:1537 invalidate_lock() warn: pointer dereferenced without being set 'mg->cell'
drivers/dma/mmp_pdma.c:486 mmp_pdma_prep_memcpy() warn: pointer dereferenced without being set 'prev'
drivers/dma/mmp_pdma.c:571 mmp_pdma_prep_slave_sg() warn: pointer dereferenced without being set 'prev'
drivers/dma/mmp_pdma.c:659 mmp_pdma_prep_dma_cyclic() warn: pointer dereferenced without being set 'prev'
drivers/dma/pch_dma.c:629 pd_prep_slave_sg() warn: pointer dereferenced without being set 'prev'
drivers/dma/mediatek/mtk-cqdma.c:535 mtk_cqdma_prep_dma_memcpy() warn: pointer dereferenced without being set 'prev_tx'
drivers/pwm/pwm-lpc32xx.c:144 lpc32xx_pwm_probe() warn: pointer dereferenced without being set 'lpc32xx->chip.pwms'
drivers/media/usb/as102/as10x_cmd_stream.c:57 as10x_cmd_add_PID_filter() warn: pointer dereferenced without being set 'prsp'
drivers/media/usb/as102/as10x_cmd_stream.c:108 as10x_cmd_del_PID_filter() warn: pointer dereferenced without being set 'prsp'
drivers/media/usb/as102/as10x_cmd_stream.c:151 as10x_cmd_start_streaming() warn: pointer dereferenced without being set 'prsp'
drivers/media/usb/as102/as10x_cmd.c:47 as10x_cmd_turn_on() warn: pointer dereferenced without being set 'prsp'
drivers/media/usb/as102/as10x_cmd.c:87 as10x_cmd_turn_off() warn: pointer dereferenced without being set 'prsp'
drivers/media/usb/as102/as10x_cmd.c:142 as10x_cmd_set_tune() warn: pointer dereferenced without being set 'prsp'
drivers/media/usb/as102/as10x_cmd.c:186 as10x_cmd_get_tune_status() warn: pointer dereferenced without being set 'prsp'
drivers/media/usb/as102/as10x_cmd.c:239 as10x_cmd_get_tps() warn: pointer dereferenced without being set 'prsp'
drivers/media/usb/as102/as10x_cmd.c:298 as10x_cmd_get_demod_stats() warn: pointer dereferenced without being set 'prsp'
drivers/media/usb/as102/as10x_cmd.c:358 as10x_cmd_get_impulse_resp() warn: pointer dereferenced without being set 'prsp'
drivers/media/usb/as102/as10x_cmd_cfg.c:59 as10x_cmd_get_context() warn: pointer dereferenced without being set 'prsp'
drivers/media/usb/as102/as10x_cmd_cfg.c:117 as10x_cmd_set_context() warn: pointer dereferenced without being set 'prsp'
drivers/media/usb/as102/as10x_cmd_cfg.c:166 as10x_cmd_eLNA_change_mode() warn: pointer dereferenced without being set 'prsp'
drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c:431 mtk_vcodec_probe() warn: pointer dereferenced without being set 'dev->m2m_dev_dec->source'
drivers/media/pci/cx18/cx18-alsa-main.c:145 snd_cx18_init() warn: pointer dereferenced without being set 'cxsc->sc'
drivers/media/pci/cx25821/cx25821-core.c:1114 cx25821_risc_field_audio() warn: pointer dereferenced without being set 'sg'
drivers/media/pci/zoran/zr36016.c:118 zr36016_basic_test() warn: pointer dereferenced without being set 'ptr->codec->master_data'
drivers/media/pci/zoran/zr36016.c:157 zr36016_init() warn: pointer dereferenced without being set 'ptr->codec->master_data'
drivers/media/pci/zoran/zr36050.c:120 zr36050_basic_test() warn: pointer dereferenced without being set 'ptr->codec->master_data'
drivers/media/pci/zoran/zr36050.c:372 zr36050_init() warn: pointer dereferenced without being set 'ptr->codec->master_data'
drivers/media/pci/zoran/zr36060.c:114 zr36060_basic_test() warn: pointer dereferenced without being set 'ptr->codec->master_data'
drivers/media/pci/zoran/zr36060.c:327 zr36060_init() warn: pointer dereferenced without being set 'ptr->codec->master_data'
drivers/media/pci/cobalt/cobalt-alsa-main.c:111 cobalt_alsa_init() warn: pointer dereferenced without being set 'cobsc->s'
drivers/media/pci/cobalt/cobalt-alsa-main.c:111 cobalt_alsa_init() warn: pointer dereferenced without being set 'cobsc->sc'
drivers/media/pci/ivtv/ivtv-alsa-main.c:144 snd_ivtv_init() warn: pointer dereferenced without being set 'itvsc->sc'
drivers/media/common/videobuf2/videobuf2-dma-contig.c:200 vb2_dc_alloc_coherent() warn: pointer dereferenced without being set 'buf->dev'
drivers/media/common/videobuf2/videobuf2-dma-contig.c:219 vb2_dc_alloc_non_coherent() warn: pointer dereferenced without being set 'buf->dev'
drivers/scsi/initio.c:2874 initio_probe_one() warn: pointer dereferenced without being set 'prev'
drivers/scsi/libiscsi.c:3779 iscsi_conn_get_addr_param() warn: pointer dereferenced without being set 'sin6'
drivers/scsi/be2iscsi/be_main.c:2561 beiscsi_alloc_mem() warn: pointer dereferenced without being set 'mem_descr->mem_array'
drivers/input/touchscreen/iqs5xx.c:926 fw_file_store() warn: pointer dereferenced without being set 'iqs5xx->input'
drivers/crypto/caam/caampkc.c:898 caam_read_rsa_crt() warn: pointer dereferenced without being set 'ptr'
drivers/crypto/caam/caampkc.c:923 caam_read_raw_data() warn: pointer dereferenced without being set 'buf'
drivers/crypto/sa2ul.c:1528 sa_sha_cra_init_alg() warn: pointer dereferenced without being set 'ctx->fallback.ahash'
drivers/tee/tee_shm_pool.c:46 pool_op_gen_destroy_pool() warn: pointer dereferenced without being set 'pool->private_data'
drivers/net/wireless/broadcom/b43/phy_n.c:4258 b43_nphy_tx_gain_table_upload() warn: pointer dereferenced without being set 'rf_pwr_offset_table'
drivers/net/wireless/ath/wcn36xx/dxe.c:89 wcn36xx_dxe_allocate_ctl_block() warn: pointer dereferenced without being set 'prev_ctl'
drivers/net/wireless/ath/wcn36xx/dxe.c:214 wcn36xx_dxe_init_descs() warn: pointer dereferenced without being set 'prev_dxe'
drivers/net/wireless/ath/ath11k/pci.c:846 ath11k_pci_probe() warn: pointer dereferenced without being set 'ab_pci->ab->pci.msi.config'
drivers/net/xen-netback/netback.c:1535 xenvif_map_frontend_data_rings() warn: pointer dereferenced without being set '&(rxs->req_prod)'
drivers/net/xen-netback/netback.c:1520 xenvif_map_frontend_data_rings() warn: pointer dereferenced without being set '&(txs->req_prod)'
drivers/net/xen-netback/interface.c:634 xenvif_connect_ctrl() warn: pointer dereferenced without being set '&(shared->req_prod)'
drivers/net/wireguard/peer.c:41 wg_peer_create() warn: pointer dereferenced without being set 'peer->handshake.static_identity'
drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c:263 dpaa2_eth_build_frag_skb() warn: pointer dereferenced without being set 'skb'
drivers/net/ethernet/freescale/fman/fman_memac.c:1016 memac_config() warn: pointer dereferenced without being set 'memac->fm'
drivers/net/ethernet/freescale/fman/fman_tgec.c:721 tgec_config() warn: pointer dereferenced without being set 'tgec->fm'
drivers/net/ethernet/netronome/nfp/abm/main.c:331 nfp_abm_vnic_alloc() warn: pointer dereferenced without being set 'alink->abm'
drivers/net/ethernet/netronome/nfp/flower/main.c:597 nfp_flower_spawn_phy_reprs() warn: pointer dereferenced without being set 'app->ctrl'
drivers/net/ethernet/netronome/nfp/bpf/jit.c:4232 nfp_bpf_opt_ldst_gather() warn: pointer dereferenced without being set 'head_ld_meta'
drivers/net/ethernet/netronome/nfp/bpf/jit.c:4211 nfp_bpf_opt_ldst_gather() warn: pointer dereferenced without being set 'head_st_meta'
drivers/net/ethernet/netronome/nfp/bpf/jit.c:4217 nfp_bpf_opt_ldst_gather() warn: pointer dereferenced without being set 'prev_ld'
drivers/net/ethernet/netronome/nfp/bpf/jit.c:4218 nfp_bpf_opt_ldst_gather() warn: pointer dereferenced without being set 'prev_st'
drivers/net/ethernet/netronome/nfp/bpf/jit.c:4358 nfp_bpf_opt_pkt_cache() warn: pointer dereferenced without being set 'range_node'
drivers/net/ethernet/pensando/ionic/ionic_txrx.c:1088 ionic_tx_tso() warn: pointer dereferenced without being set 'elem'
drivers/net/ethernet/marvell/skge.c:2633 skge_up() warn: pointer dereferenced without being set 'skge->rx_ring.start->desc'
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c:2186 mlx5dr_action_destroy() warn: pointer dereferenced without being set 'action->aso'
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c:2189 mlx5dr_action_destroy() warn: pointer dereferenced without being set 'action->range'
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c:2162 mlx5dr_action_destroy() warn: pointer dereferenced without being set 'action->reformat'
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c:2165 mlx5dr_action_destroy() warn: pointer dereferenced without being set 'action->rewrite'
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c:2183 mlx5dr_action_destroy() warn: pointer dereferenced without being set 'action->sampler'
drivers/net/ethernet/mellanox/mlx5/core/en/tc/act/vlan.c:75 parse_tc_vlan_action() warn: pointer dereferenced without being set 'parse_state'
drivers/net/ethernet/cavium/thunder/nicvf_queues.c:1714 nicvf_get_rcv_skb() warn: pointer dereferenced without being set 'skb'
drivers/net/ethernet/emulex/benet/be_cmds.c:1370 be_cmd_txq_create() warn: pointer dereferenced without being set '&req->hdr'
drivers/net/ethernet/emulex/benet/be_cmds.c:1543 be_cmd_if_create() warn: pointer dereferenced without being set '&req->hdr'
drivers/net/ethernet/emulex/benet/be_cmds.c:1576 be_cmd_if_destroy() warn: pointer dereferenced without being set '&req->hdr'
drivers/net/ethernet/emulex/benet/be_cmds.c:1778 be_cmd_get_fat_dump_len() warn: pointer dereferenced without being set '&req->hdr'
drivers/net/ethernet/intel/i40e/i40e_main.c:2320 i40e_update_filter_state() warn: pointer dereferenced without being set 'add_head'
drivers/net/vxlan/vxlan_vnifilter.c:379 vxlan_vnifilter_dump_dev() warn: pointer dereferenced without being set 'vend'
drivers/misc/genwqe/card_utils.c:417 genwqe_setup_sgl() warn: pointer dereferenced without being set 'last_s'
drivers/misc/bcm-vk/bcm_vk_msg.c:1055 bcm_vk_read() warn: pointer dereferenced without being set 'entry'
drivers/base/regmap/regmap-spi-avmm.c:449 br_do_rx_and_pkt_phy_parse() warn: pointer dereferenced without being set 'tb'
drivers/soc/qcom/llcc-qcom.c:641 llcc_slice_activate() warn: pointer dereferenced without being set 'drv_data->bitmap'
drivers/soc/qcom/llcc-qcom.c:681 llcc_slice_deactivate() warn: pointer dereferenced without being set 'drv_data->bitmap'
drivers/soc/fsl/qbman/qman_test_stash.c:222 allocate_frame_data() warn: pointer dereferenced without being set 'qman_dma_portal'
block/blk-integrity.c:82 blk_rq_map_integrity_sg() warn: pointer dereferenced without being set 'sg'
security/integrity/ima/ima_main.c:1000 process_buffer_measurement() warn: pointer dereferenced without being set 'entry->template_desc'
security/integrity/ima/ima_api.c:155 ima_add_violation() warn: pointer dereferenced without being set 'entry->template_desc'
security/integrity/ima/ima_api.c:375 ima_store_measurement() warn: pointer dereferenced without being set 'entry->template_desc'
security/integrity/ima/ima_init.c:86 ima_add_boot_aggregate() warn: pointer dereferenced without being set 'entry->template_desc'
fs/btrfs/tests/free-space-tests.c:362 check_cache_empty() warn: pointer dereferenced without being set 'cache->free_space_ctl'
fs/btrfs/tests/free-space-tests.c:426 test_steal_space_from_bitmap_to_extent() warn: pointer dereferenced without being set 'cache->free_space_ctl'
fs/btrfs/extent_map.c:118 tree_insert() warn: pointer dereferenced without being set 'entry'
fs/ntfs/aops.c:1086 ntfs_write_mst_block() warn: pointer dereferenced without being set 'rec_start_bh'
fs/xfs/scrub/agheader_repair.c:761 xrep_agfl() warn: pointer dereferenced without being set 'sc->tp'
fs/xfs/scrub/rmap.c:552 xchk_rmapbt() warn: pointer dereferenced without being set 'sc->sa.rmap_cur'
fs/xfs/scrub/refcount.c:520 xchk_refcountbt() warn: pointer dereferenced without being set 'sc->sa.refc_cur'
fs/xfs/libxfs/xfs_dir2_node.c:668 xfs_dir2_leafn_lookup_for_addname() warn: pointer dereferenced without being set 'curbp'
fs/xfs/libxfs/xfs_dir2_node.c:886 xfs_dir2_leafn_lookup_for_entry() warn: pointer dereferenced without being set 'state->extrablk.bp'
fs/xfs/libxfs/xfs_da_btree.c:1150 xfs_da3_join() warn: pointer dereferenced without being set 'drop_blk->bp'
fs/ext2/dir.c:531 ext2_add_link() warn: pointer dereferenced without being set 'de'
fs/gfs2/xattr.c:1350 ea_dealloc_indirect() warn: pointer dereferenced without being set 'rgd'
fs/gfs2/bmap.c:1467 sweep_bh_for_rgrps() warn: pointer dereferenced without being set 'rd_gh->gh_gl'
fs/ecryptfs/crypto.c:1370 ecryptfs_read_metadata() warn: pointer dereferenced without being set 'ecryptfs_header_cache'
fs/ecryptfs/crypto.c:1601 ecryptfs_add_new_key_tfm() warn: pointer dereferenced without being set 'ecryptfs_key_tfm_cache'
fs/ecryptfs/keystore.c:1263 parse_tag_1_packet() warn: pointer dereferenced without being set 'ecryptfs_auth_tok_list_item_cache'
fs/ecryptfs/keystore.c:1398 parse_tag_3_packet() warn: pointer dereferenced without being set 'ecryptfs_auth_tok_list_item_cache'
fs/ecryptfs/keystore.c:2416 ecryptfs_generate_key_packet_set() warn: pointer dereferenced without being set 'ecryptfs_key_record_cache'
fs/ecryptfs/keystore.c:2497 ecryptfs_add_keysig() warn: pointer dereferenced without being set 'ecryptfs_key_sig_cache'
fs/ecryptfs/keystore.c:2517 ecryptfs_add_global_auth_tok() warn: pointer dereferenced without being set 'ecryptfs_global_auth_tok_cache'
fs/ecryptfs/inode.c:327 ecryptfs_lookup_interpose() warn: pointer dereferenced without being set 'ecryptfs_dentry_info_cache'
fs/ecryptfs/main.c:595 ecryptfs_mount() warn: pointer dereferenced without being set 'ecryptfs_dentry_info_cache'
fs/ecryptfs/main.c:489 ecryptfs_mount() warn: pointer dereferenced without being set 'ecryptfs_sb_info_cache'
fs/ecryptfs/file.c:195 ecryptfs_open() warn: pointer dereferenced without being set 'ecryptfs_file_info_cache'
fs/ecryptfs/file.c:263 ecryptfs_dir_open() warn: pointer dereferenced without being set 'ecryptfs_file_info_cache'
fs/ecryptfs/super.c:41 ecryptfs_alloc_inode() warn: pointer dereferenced without being set 'ecryptfs_inode_info_cache'
fs/ecryptfs/mmap.c:420 ecryptfs_write_inode_size_to_xattr() warn: pointer dereferenced without being set 'ecryptfs_xattr_cache'
fs/namespace.c:2274 attach_recursive_mnt() warn: pointer dereferenced without being set 'child'
fs/ceph/caps.c:3933 handle_cap_export() warn: pointer dereferenced without being set 'tcap'
fs/overlayfs/namei.c:868 ovl_lookup() warn: pointer dereferenced without being set 'd.name.name'
fs/overlayfs/super.c:831 ovl_workdir_create() warn: pointer dereferenced without being set 'work->d_inode'
fs/ubifs/lpt.c:1444 ubifs_get_pnode() warn: pointer dereferenced without being set 'branch->pnode'
fs/ocfs2/dlm/dlmmaster.c:856 dlm_get_lock_resource() warn: pointer dereferenced without being set 'mle'
fs/ocfs2/dir.c:1536 ocfs2_remove_block_from_free_list() warn: pointer dereferenced without being set 'bh'
fs/ocfs2/dir.c:1617 __ocfs2_add_entry() warn: pointer dereferenced without being set 'bh'
fs/coredump.c:777 do_coredump() warn: pointer dereferenced without being set 'cprm.file'
fs/f2fs/segment.c:1049 __lookup_discard_cmd_ret() warn: pointer dereferenced without being set 'dc'
fs/f2fs/extent_cache.c:256 __lookup_extent_node_ret() warn: pointer dereferenced without being set 'en'
kernel/bpf/btf.c:5638 btf_get_prog_ctx_type() warn: pointer dereferenced without being set 'btf_vmlinux'
kernel/bpf/btf.c:5699 get_kern_ctx_btf_id() warn: pointer dereferenced without being set 'btf_vmlinux'
kernel/bpf/bpf_struct_ops.c:322 check_zero_holes() warn: pointer dereferenced without being set 'btf_vmlinux'
kernel/bpf/bpf_struct_ops.c:429 bpf_struct_ops_map_update_elem() warn: pointer dereferenced without being set 'btf_vmlinux'
kernel/bpf/bpf_struct_ops.c:601 bpf_struct_ops_map_seq_show_elem() warn: pointer dereferenced without being set 'btf_vmlinux'
kernel/bpf/verifier.c:5639 check_ptr_to_map_access() warn: pointer dereferenced without being set 'btf_vmlinux'
kernel/bpf/verifier.c:7230 check_reg_type() warn: pointer dereferenced without being set 'btf_vmlinux'
kernel/bpf/verifier.c:18334 check_struct_ops_btf_id() warn: pointer dereferenced without being set 'btf_vmlinux'
kernel/bpf/core.c:2472 bpf_prog_array_copy() warn: pointer dereferenced without being set 'old_array'
sound/core/seq/seq_prioq.c:302 snd_seq_prioq_leave() warn: pointer dereferenced without being set 'freeprev'
sound/core/seq/seq_prioq.c:292 snd_seq_prioq_leave() warn: pointer dereferenced without being set 'prev'
sound/core/seq/seq_prioq.c:417 snd_seq_prioq_remove_events() warn: pointer dereferenced without being set 'freeprev'
sound/core/seq/seq_prioq.c:405 snd_seq_prioq_remove_events() warn: pointer dereferenced without being set 'prev'
sound/pci/ctxfi/ctdaio.c:534 get_daio_rsc() warn: pointer dereferenced without being set 'mgr->mgr.rscs'
sound/pci/ctxfi/ctdaio.c:590 put_daio_rsc() warn: pointer dereferenced without being set 'mgr->mgr.rscs'
sound/soc/codecs/ad1980.c:211 ad1980_reset() warn: pointer dereferenced without being set 'ac97->bus'
sound/soc/codecs/max98088.c:1597 max98088_handle_eq_pdata() warn: pointer dereferenced without being set 'max98088->eq_texts'
sound/soc/codecs/max98095.c:1594 max98095_handle_eq_pdata() warn: pointer dereferenced without being set 'max98095->eq_texts'
sound/soc/codecs/max98095.c:1746 max98095_handle_bq_pdata() warn: pointer dereferenced without being set 'max98095->bq_texts'
sound/soc/codecs/wm8904.c:2017 wm8904_handle_retune_mobile_pdata() warn: pointer dereferenced without being set 'wm8904->retune_mobile_texts'
sound/soc/codecs/wm8994.c:3374 wm8994_handle_retune_mobile_pdata() warn: pointer dereferenced without being set 'wm8994->retune_mobile_texts'
sound/soc/codecs/wm8996.c:2568 wm8996_retune_mobile_pdata() warn: pointer dereferenced without being set 'wm8996->retune_mobile_texts'
sound/soc/sh/fsi.c:2028 fsi_probe() warn: pointer dereferenced without being set 'master->fsia.playback.handler'
sound/soc/intel/avs/pcm.c:535 avs_dai_fe_hw_params() warn: pointer dereferenced without being set 'data->path'
sound/soc/intel/skylake/skl-topology.c:2488 skl_tplg_get_token() warn: pointer dereferenced without being set 'mconfig->pipe'
net/dccp/ipv6.c:352 dccp_v6_conn_request() warn: pointer dereferenced without being set 'dccp6_request_sock_ops.slab'
net/dccp/ipv4.c:622 dccp_v4_conn_request() warn: pointer dereferenced without being set 'dccp_request_sock_ops.slab'
net/core/skbuff.c:4467 skb_segment() warn: pointer dereferenced without being set 'tail'
net/sctp/stream_interleave.c:787 sctp_intl_retrieve_first_uo() warn: pointer dereferenced without being set 'sin'
net/sctp/stream_interleave.c:894 sctp_intl_retrieve_first() warn: pointer dereferenced without being set 'sin'
net/mac80211/mesh_pathtbl.c:560 mesh_fast_tx_cache() warn: pointer dereferenced without being set 'hdr'
net/bridge/br_vlan.c:2029 br_vlan_dump_dev() warn: pointer dereferenced without being set 'range_end'
net/bridge/br_vlan_options.c:307 br_vlan_process_options() warn: pointer dereferenced without being set 'curr_end'
net/bridge/br_vlan_options.c:713 br_vlan_rtm_process_global_options() warn: pointer dereferenced without being set 'curr_end'
net/bridge/br_netlink_tunnel.c:51 __get_num_vlan_tunnel_infos() warn: pointer dereferenced without being set 'vtend'
net/bridge/br_netlink_tunnel.c:167 br_fill_vlan_tunnel_info() warn: pointer dereferenced without being set 'vtend'
net/ipv6/ip6_input.c:128 ip6_list_rcv_finish() warn: pointer dereferenced without being set 'hint'
net/xfrm/xfrm_device.c:219 validate_xmit_xfrm() warn: pointer dereferenced without being set 'pskb'
net/ipv4/fib_semantics.c:2093 fib_select_default() warn: pointer dereferenced without being set 'fa1'
net/rds/connection.c:307 __rds_conn_create() warn: pointer dereferenced without being set 'conn'
lib/maple_tree.c:3848 mas_extend_spanning_null() warn: pointer dereferenced without being set 'l_wr_mas->slots'
lib/maple_tree.c:3863 mas_extend_spanning_null() warn: pointer dereferenced without being set 'r_wr_mas->slots'
lib/sg_split.c:87 sg_split_phys() warn: pointer dereferenced without being set 'out_sg'
mm/gup.c:1633 __mm_populate() warn: pointer dereferenced without being set 'vma'
mm/zsmalloc.c:1092 create_page_chain() warn: pointer dereferenced without being set 'prev_page'
^ permalink raw reply
* Re: [REGRESSION] Bug 217353 - Logitech MX Anywhere 3 scroll wheel works incorrectly over USB dongle
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-04-22 8:09 UTC (permalink / raw)
To: Bastien Nocera
Cc: Benjamin Tissoires, Jiri Kosina, open list:HID CORE LAYER, LKML,
Linux kernel regressions list
In-Reply-To: <5fa67291-98d0-c8d5-ca71-5a86b9adff41@leemhuis.info>
Lo!
On 21.04.23 11:11, Linux regression tracking (Thorsten Leemhuis) wrote:
>
> Bastien, from the bug report it to me sounds like it might be another
> case caused by 908d325e166 ("HID: logitech-hidpp: Detect hi-res
> scrolling support"), but I might be totally wrong with that (and the
> reporter didn't even explicitly state that it's a regression, but it
> sounds like one). If I got something wrong I apologize in advance.
>
> Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=217353 :
Ignore that mail, user in between noticed it's a userspace problem.
Sorry for the noise.
#regzbot resolved: userspace problem according to the reporter
#regzbot ignore-activity
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
^ permalink raw reply
* Re: [PATCH] dt-bindings: input: touchscreen: goodix: clarify irq-gpios misleading text
From: Rob Herring @ 2023-04-21 20:55 UTC (permalink / raw)
To: Luca Ceresoli
Cc: Jarrah Gosbell, Dmitry Torokhov, Thomas Petazzoni, devicetree,
Rob Herring, linux-kernel, linux-input, Stephen Boyd,
Krzysztof Kozlowski
In-Reply-To: <20230419171849.946986-1-luca.ceresoli@bootlin.com>
On Wed, 19 Apr 2023 19:18:49 +0200, Luca Ceresoli wrote:
> The irq-gpios description misleading, apparently saying that driving the
> IRQ GPIO resets the device, which is even more puzzling as there is a reset
> GPIO as well.
>
> In reality the IRQ pin can be driven during the reset sequence to configure
> the client address, as it becomes clear after checking both the datasheet
> and the driver code. Improve the text to clarify that.
>
> Also rephrase to remove reference to the driver, which is not appropriate
> in the bindings.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
> .../devicetree/bindings/input/touchscreen/goodix.yaml | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 5/5] arm64: dts: qcom: sdm845-shift-axolotl: update focaltech touchscreen properties
From: Caleb Connolly @ 2023-04-21 16:02 UTC (permalink / raw)
To: Joel Selvaraj, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Andy Gross, Bjorn Andersson, Konrad Dybcio, Henrik Rydberg,
Arnd Bergmann, Robert Jarzmik, Jeff LaBundy, Markuss Broks,
Jean Delvare, Max Krummenacher, Chris Morgan, Job Noorman,
Alistair Francis, Hans de Goede, Maxime Ripard
Cc: linux-input, devicetree, linux-kernel, linux-arm-msm,
~postmarketos/upstreaming, phone-devel
In-Reply-To: <20230415020222.216232-6-joelselvaraj.oss@gmail.com>
On 15/04/2023 03:02, Joel Selvaraj wrote:
> The touchscreen nodes were added before the driver patches were merged.
> Update the focaltech touchscreen properties to match with the upstreamed
> focaltech driver. Also, the touchscreen used is in axolotl is fts5452
> and not fts8719.
>
> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
Reviewed-by: Caleb Connolly <caleb@connolly.tech>
> ---
> .../boot/dts/qcom/sdm845-shift-axolotl.dts | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts b/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts
> index b54e304abf71..70286e53e000 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts
> @@ -474,23 +474,21 @@ &i2c5 {
> status = "okay";
>
> touchscreen@38 {
> - compatible = "focaltech,fts8719";
> + compatible = "focaltech,fts5452";
> reg = <0x38>;
> - wakeup-source;
> - interrupt-parent = <&tlmm>;
> - interrupts = <125 0x2>;
> - vdd-supply = <&vreg_l28a_3p0>;
> - vcc-i2c-supply = <&vreg_l14a_1p88>;
>
> - pinctrl-names = "default", "suspend";
> + interrupts-extended = <&tlmm 125 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&tlmm 99 GPIO_ACTIVE_LOW>;
> +
> + avdd-supply = <&vreg_l28a_3p0>;
> + vddio-supply = <&vreg_l14a_1p88>;
> +
> pinctrl-0 = <&ts_int_active &ts_reset_active>;
> pinctrl-1 = <&ts_int_suspend &ts_reset_suspend>;
> + pinctrl-names = "default", "suspend";
>
> - reset-gpio = <&tlmm 99 GPIO_ACTIVE_HIGH>;
> - irq-gpio = <&tlmm 125 GPIO_TRANSITORY>;
> touchscreen-size-x = <1080>;
> touchscreen-size-y = <2160>;
> - focaltech,max-touch-number = <5>;
> };
> };
>
> --
> 2.40.0
>
--
Kind Regards,
Caleb
^ permalink raw reply
* [BUG] sysfs: cannot create duplicate filename, with Lenovo keyboard
From: Frédéric Lamorce @ 2023-04-21 12:53 UTC (permalink / raw)
To: linux-input
In-Reply-To: <CAPNj2HJdA6uZ9xvvHsVNAFjS88-xALwOCXT1Hv4A5AG-HismAQ@mail.gmail.com>
kernel warning about a duplicate filename in sysfs after a resume.
CPU: AMD 5600H
distro : MX-21.3_ahs_x64 Wildflower September 18 2022 base: Debian
GNU/Linux 11 (bullseye)
kernel tested: 6.1.0-6mx-amd64 ; 6.1.0-7mx-amd64 ;
6.2.10-1-liquorix-amd64 ; 6.2.11-3-liquorix-amd64
I am using a Lenovo TrackPoint Keyboard II, it's a keyboard with an
integrated trackpad. It is paired in bluetooth mode. It is using the
hid-lenovo module.
Bluetooth is using mediatek mt7921e and btusb
After a suspend/resume, sometimes, there is a kernel warning in dmesg.
Maybe because the keyboard and mouse are using the same MAC address
and it tries to re-create the battery info?
[22706.169919] sysfs: cannot create duplicate filename
'/class/power_supply/hid-d8:ab:24:63:e7:77-battery'
[22706.169928] CPU: 2 PID: 41456 Comm: kworker/2:1 Tainted: P
O 6.2.11-3-liquorix-amd64 #1 liquorix 6.2-16~mx21+1
[22706.169934] Hardware name: AZW SER/SER, BIOS V1.15_P4C7M43_Blink 09/17/2022
[22706.169937] Workqueue: events uhid_device_add_worker [uhid]
[22706.169955] Call Trace:
[22706.169960] <TASK>
[22706.169962] dump_stack_lvl+0x44/0x60
[22706.169973] sysfs_warn_dup+0x57/0x70
[22706.169979] sysfs_create_link+0xc2/0xd0
[22706.169983] device_add+0x208/0xb70
[22706.169991] __power_supply_register+0x27b/0x420
[22706.169999] hidinput_setup_battery.isra.0+0x19e/0x240
[22706.170007] hidinput_connect+0x1326/0x4e60
[22706.170016] hid_connect+0x581/0x5d0
[22706.170021] hid_hw_start+0x41/0x70
[22706.170025] lenovo_probe+0x30/0x670 [hid_lenovo]
[22706.170038] hid_device_probe+0xf4/0x190
[22706.170044] really_probe+0x1d6/0x3c0
[22706.170050] __driver_probe_device+0x72/0xe0
[22706.170054] driver_probe_device+0x1e/0x170
[22706.170058] __device_attach_driver+0xac/0x160
[22706.170063] ? __pfx___device_attach_driver+0x10/0x10
[22706.170068] bus_for_each_drv+0x7e/0xd0
[22706.170073] device_initial_probe+0xfd/0x1e0
[22706.170079] bus_probe_device+0xa8/0xc0
[22706.170083] device_add+0x3ec/0xb70
[22706.170088] hid_add_device+0xf8/0x3b0
[22706.170092] ? od_dbs_update+0xb9/0x190
[22706.170099] uhid_device_add_worker+0x15/0x60 [uhid]
[22706.170108] process_one_work+0x1dc/0x3e0
[22706.170115] worker_thread+0x4d/0x460
[22706.170121] ? __pfx_worker_thread+0x10/0x10
[22706.170126] kthread+0xd8/0x100
[22706.170131] ? __pfx_kthread+0x10/0x10
[22706.170134] ret_from_fork+0x2c/0x50
[22706.170142] </TASK>
[22706.170168] lenovo 0005:17EF:60E1.001D: can't register power supply: -17
[22706.170305] input: TrackPoint Keyboard II as
/devices/virtual/misc/uhid/0005:17EF:60E1.001D/input/input42
[22706.171437] lenovo 0005:17EF:60E1.001D: input,hidraw4: BLUETOOTH
HID v0.47 Keyboard [TrackPoint Keyboard II] on 4c:d5:77:f0:c3:6c
[22734.816970] Bluetooth: hci0: ACL packet for unknown connection handle 512
[22735.849582] input: SANWA Gravi Mouse as
/devices/virtual/misc/uhid/0005:1915:0040.001E/input/input43
[22735.850012] input: SANWA Gravi Consumer Control as
/devices/virtual/misc/uhid/0005:1915:0040.001E/input/input44
[22735.851199] hid-generic 0005:1915:0040.001E: input,hidraw2:
BLUETOOTH HID v0.01 Mouse [SANWA Gravi] on 4c:d5:77:f0:c3:6c
[22760.786365] sysfs: cannot create duplicate filename
'/class/power_supply/hid-d8:ab:24:63:e7:77-battery'
[22760.786372] CPU: 6 PID: 37516 Comm: kworker/6:2 Tainted: P
O 6.2.11-3-liquorix-amd64 #1 liquorix 6.2-16~mx21+1
[22760.786378] Hardware name: AZW SER/SER, BIOS V1.15_P4C7M43_Blink 09/17/2022
[22760.786380] Workqueue: events uhid_device_add_worker [uhid]
[22760.786396] Call Trace:
[22760.786400] <TASK>
[22760.786402] dump_stack_lvl+0x44/0x60
[22760.786411] sysfs_warn_dup+0x57/0x70
[22760.786416] sysfs_create_link+0xc2/0xd0
[22760.786419] device_add+0x208/0xb70
[22760.786427] __power_supply_register+0x27b/0x420
[22760.786434] hidinput_setup_battery.isra.0+0x19e/0x240
[22760.786441] hidinput_connect+0x1326/0x4e60
[22760.786449] hid_connect+0x581/0x5d0
[22760.786454] hid_hw_start+0x41/0x70
[22760.786459] lenovo_probe+0x30/0x670 [hid_lenovo]
[22760.786469] hid_device_probe+0xf4/0x190
[22760.786474] really_probe+0x1d6/0x3c0
[22760.786478] __driver_probe_device+0x72/0xe0
[22760.786483] driver_probe_device+0x1e/0x170
[22760.786487] __device_attach_driver+0xac/0x160
[22760.786491] ? __pfx___device_attach_driver+0x10/0x10
[22760.786495] bus_for_each_drv+0x7e/0xd0
[22760.786499] device_initial_probe+0xfd/0x1e0
[22760.786504] bus_probe_device+0xa8/0xc0
[22760.786507] device_add+0x3ec/0xb70
[22760.786513] hid_add_device+0xf8/0x3b0
[22760.786517] uhid_device_add_worker+0x15/0x60 [uhid]
[22760.786526] process_one_work+0x1dc/0x3e0
[22760.786534] worker_thread+0x4d/0x460
[22760.786540] ? __pfx_worker_thread+0x10/0x10
[22760.786545] kthread+0xd8/0x100
[22760.786549] ? __pfx_kthread+0x10/0x10
[22760.786553] ret_from_fork+0x2c/0x50
[22760.786561] </TASK>
[22760.786575] lenovo 0005:17EF:60E1.001F: can't register power supply: -17
[22760.786781] input: TrackPoint Keyboard II as
/devices/virtual/misc/uhid/0005:17EF:60E1.001F/input/input45
[22760.787191] lenovo 0005:17EF:60E1.001F: input,hidraw4: BLUETOOTH
HID v0.47 Keyboard [TrackPoint Keyboard II] on 4c:d5:77:f0:c3:6c
^ permalink raw reply
* [REGRESSION] Bug 217353 - Logitech MX Anywhere 3 scroll wheel works incorrectly over USB dongle
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-04-21 9:11 UTC (permalink / raw)
To: Bastien Nocera
Cc: Benjamin Tissoires, Jiri Kosina, open list:HID CORE LAYER, LKML,
Linux kernel regressions list
Hi, Thorsten here, the Linux kernel's regression tracker.
I noticed a regression report in bugzilla.kernel.org. As many (most?)
kernel developers don't keep an eye on it, I decided to forward it by mail.
Bastien, from the bug report it to me sounds like it might be another
case caused by 908d325e166 ("HID: logitech-hidpp: Detect hi-res
scrolling support"), but I might be totally wrong with that (and the
reporter didn't even explicitly state that it's a regression, but it
sounds like one). If I got something wrong I apologize in advance.
Note, you have to use bugzilla to reach the reporter, as I sadly[1] can
not CCed them in mails like this.
Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=217353 :
> vova7890 2023-04-18 22:40:07 UTC
>
> Need a solution similar as for
> https://bugzilla.kernel.org/show_bug.cgi?id=216885 but for the usb dongle.
>
> hid_is_usb(hidpp->hid_dev) always return 0 for the usb dongle. Through
> bluetooth it's working fine as hid_logitech_hidpp is not loading for
> this mouse with over BT mode.
See the ticket for more details.
[TLDR for the rest of this mail: I'm adding this report to the list of
tracked Linux kernel regressions; the text you find below is based on a
few templates paragraphs you might have encountered already in similar
form.]
BTW, let me use this mail to also add the report to the list of tracked
regressions to ensure it's doesn't fall through the cracks:
#regzbot introduced: 908d325e1665
https://bugzilla.kernel.org/show_bug.cgi?id=217353
#regzbot title: hid: Logitech MX Anywhere 3 scroll wheel works
incorrectly over USB dongle
#regzbot ignore-activity
This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply and tell me -- ideally
while also telling regzbot about it, as explained by the page listed in
the footer of this mail.
Developers: When fixing the issue, remember to add 'Link:' tags pointing
to the report (e.g. the buzgzilla ticket and maybe this mail as well, if
this thread sees some discussion). See page linked in footer for details.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
[1] because bugzilla.kernel.org tells users upon registration their
"email address will never be displayed to logged out users"
^ permalink raw reply
* [PATCH] Input: cyttsp4_core - change del_timer_sync() to timer_shutdown_sync()
From: Duoming Zhou @ 2023-04-21 8:29 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, linus.walleij, dmitry.torokhov, Duoming Zhou
The watchdog_timer can schedule tx_timeout_task and watchdog_work
can also arm watchdog_timer. The process is shown below:
----------- timer schedules work ------------
cyttsp4_watchdog_timer() //timer handler
schedule_work(&cd->watchdog_work)
----------- work arms timer ------------
cyttsp4_watchdog_work() //workqueue callback function
cyttsp4_start_wd_timer()
mod_timer(&cd->watchdog_timer, ...)
Although del_timer_sync() and cancel_work_sync() are called in
cyttsp4_remove(), the timer and workqueue could still be rearmed.
As a result, the possible use after free bugs could happen. The
process is shown below:
(cleanup routine) | (timer and workqueue routine)
cyttsp4_remove() | cyttsp4_watchdog_timer() //timer
cyttsp4_stop_wd_timer() | schedule_work()
del_timer_sync() |
| cyttsp4_watchdog_work() //worker
| cyttsp4_start_wd_timer()
| mod_timer()
cancel_work_sync() |
| cyttsp4_watchdog_timer() //timer
| schedule_work()
del_timer_sync() |
kfree(cd) //FREE |
| cyttsp4_watchdog_work() // reschedule!
| cd-> //USE
This patch changes del_timer_sync() to timer_shutdown_sync(),
which could prevent rearming of the timer from the workqueue.
Fixes: 17fb1563d69b ("Input: cyttsp4 - add core driver for Cypress TMA4XX touchscreen devices")
Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
---
drivers/input/touchscreen/cyttsp4_core.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/cyttsp4_core.c b/drivers/input/touchscreen/cyttsp4_core.c
index 0cd6f626ade..7cb26929dc7 100644
--- a/drivers/input/touchscreen/cyttsp4_core.c
+++ b/drivers/input/touchscreen/cyttsp4_core.c
@@ -1263,9 +1263,8 @@ static void cyttsp4_stop_wd_timer(struct cyttsp4 *cd)
* Ensure we wait until the watchdog timer
* running on a different CPU finishes
*/
- del_timer_sync(&cd->watchdog_timer);
+ timer_shutdown_sync(&cd->watchdog_timer);
cancel_work_sync(&cd->watchdog_work);
- del_timer_sync(&cd->watchdog_timer);
}
static void cyttsp4_watchdog_timer(struct timer_list *t)
--
2.17.1
^ permalink raw reply related
* Proposal: Add a means to disable kernel driver logic when associated hidraw is opened
From: Vicki Pfau @ 2023-04-21 2:36 UTC (permalink / raw)
To: open list:HID CORE LAYER, Benjamin Tissoires; +Cc: Pierre-Loup Griffais
Hello,
Following up on a conversation from last year (cf. https://lore.kernel.org/linux-input/CAO-hwJLfY+D0NyCUCncrjcXETKwOBqj1CuHtB_mtGhYfKV0Bww@mail.gmail.com/) about how to approach differing opinions about how drivers should work between userspace and kernel, we're looking for a way to effectively stop the kernel from doing anything "smart" with a a HID device, e.g. a hid-sony device, when the associated hidraw is opened. At the moment, we have a specialized daemon that will find the mouse evdev associated with the controller when it's created and inhibit it, but this is anything but ideal and only handles the mouse itself, not the kernel logic in general. You can also see the hackish way this is implemented in the hid-steam driver, which uses an intermediary hid ll_driver to intercept the open and close commands and stop passing data if the hidraw gets opened. I consider this implementation to be very much a hack and an anti-pattern, and I think other HID implementers would agree, but I'm not sure there's a cleaner way to detect this in the kernel at the moment.
I see a bit of a difference of opinion as to what should be happening here between developers on our end and the kernel end. Our position is "we have a userspace driver that does everything we want, we'd like the kernel to stop trying to be smart when our driver is active", and I expect the kernel developer opinion is "why should you have a userspace driver at all when our kernel driver is fine, and if it's not you can upstream patches?", so there's probably some need to find a middle ground if we want things to work well for users in the end.
We'd previously discussed an ioctl for the hidraw, or perhaps an eBPF approach, though I think at the time of that discussion, eBPF wasn't mature for the HID subsystem. I don't know the current state of that, or if it's possible to do this with just write access to the hidraw device (the ideal case for how we want to handle this--root is pretty much out of the question for an "ideal case"). Since we still don't have a good answer for this, as far as I'm aware, I'd like to try to reach an approach that's amicable for both sides.
I know that the ioctl approach meant having to introduce logic for programs in userspace, but in this specific case, and possibly others, that's actually what we're looking for. Further, the reason root is out is because Steam isn't the OS (in most cases), and we want this to only happen when Steam is running. Having to run a daemon as root underneath Steam would require something like a setuid binary or custom sudo/polkit rules, which of course need root to set up anyway. So while eBPF may fit some use cases, e.g. full control over the system via DE or systemd, etc, it's not what we're looking for here.
Does anyone have opinions on how to proceed from here?
Vicki
^ permalink raw reply
* [PATCH RESEND v2 0/2] Input: atmel_mxt_ts - support capacitive keys
From: André Apitzsch @ 2023-04-20 21:03 UTC (permalink / raw)
To: Nick Dyer, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Linus Walleij
Cc: linux-input, devicetree, linux-arm-kernel, linux-kernel,
André Apitzsch, Krzysztof Kozlowski
Add support for touch keys found in some Atmel touch controller
configurations.
Signed-off-by: André Apitzsch <git@apitzsch.eu>
---
Changes in v2:
* Added A-b, R-b tags
---
André Apitzsch (2):
dt-bindings: input: atmel,maxtouch: add linux,keycodes
Input: atmel_mxt_ts - support capacitive keys
.../devicetree/bindings/input/atmel,maxtouch.yaml | 7 ++
drivers/input/touchscreen/atmel_mxt_ts.c | 85 ++++++++++++++++++++++
2 files changed, 92 insertions(+)
---
base-commit: f2afccfefe7be1f7346564fe619277110d341f9b
change-id: 20230407-atmel_keys-7a49c6b677b2
Best regards,
--
André Apitzsch <git@apitzsch.eu>
^ permalink raw reply
* [PATCH RESEND v2 1/2] dt-bindings: input: atmel,maxtouch: add linux,keycodes
From: André Apitzsch @ 2023-04-20 21:03 UTC (permalink / raw)
To: Nick Dyer, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Linus Walleij
Cc: linux-input, devicetree, linux-arm-kernel, linux-kernel,
André Apitzsch, Krzysztof Kozlowski
In-Reply-To: <20230407-atmel_keys-v2-0-e7b016886109@apitzsch.eu>
In some configurations the touch controller can support the touch keys.
Document the linux,keycodes property that enables those keys and
specifies the keycodes that should be used to report the key events.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: André Apitzsch <git@apitzsch.eu>
---
Documentation/devicetree/bindings/input/atmel,maxtouch.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml b/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
index 3ec579d63570..c40799355ed7 100644
--- a/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
+++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
@@ -14,6 +14,9 @@ description: |
Atmel maXTouch touchscreen or touchpads such as the mXT244
and similar devices.
+allOf:
+ - $ref: input.yaml#
+
properties:
compatible:
const: atmel,maxtouch
@@ -60,6 +63,10 @@ properties:
or experiment to determine which bit corresponds to which input. Use
KEY_RESERVED for unused padding values.
+ linux,keycodes:
+ minItems: 1
+ maxItems: 8
+
atmel,wakeup-method:
$ref: /schemas/types.yaml#/definitions/uint32
description: |
--
2.40.0
^ permalink raw reply related
* [PATCH RESEND v2 2/2] Input: atmel_mxt_ts - support capacitive keys
From: André Apitzsch @ 2023-04-20 21:03 UTC (permalink / raw)
To: Nick Dyer, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Linus Walleij
Cc: linux-input, devicetree, linux-arm-kernel, linux-kernel,
André Apitzsch
In-Reply-To: <20230407-atmel_keys-v2-0-e7b016886109@apitzsch.eu>
Add support for touch keys found in some Atmel touch controller
configurations.
Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: André Apitzsch <git@apitzsch.eu>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 85 ++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 996bf434e1cb..eb368dd1abf0 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -55,6 +55,7 @@
#define MXT_TOUCH_KEYARRAY_T15 15
#define MXT_TOUCH_PROXIMITY_T23 23
#define MXT_TOUCH_PROXKEY_T52 52
+#define MXT_TOUCH_PTC_KEYS_T97 97
#define MXT_PROCI_GRIPFACE_T20 20
#define MXT_PROCG_NOISE_T22 22
#define MXT_PROCI_ONETOUCH_T24 24
@@ -326,9 +327,13 @@ struct mxt_data {
u16 T71_address;
u8 T9_reportid_min;
u8 T9_reportid_max;
+ u8 T15_reportid_min;
+ u8 T15_reportid_max;
u16 T18_address;
u8 T19_reportid;
u16 T44_address;
+ u8 T97_reportid_min;
+ u8 T97_reportid_max;
u8 T100_reportid_min;
u8 T100_reportid_max;
@@ -344,6 +349,9 @@ struct mxt_data {
u32 *t19_keymap;
unsigned int t19_num_keys;
+ u32 *t15_keymap;
+ unsigned int t15_num_keys;
+
enum mxt_suspend_mode suspend_mode;
u32 wakeup_method;
@@ -375,6 +383,7 @@ static bool mxt_object_readable(unsigned int type)
case MXT_TOUCH_KEYARRAY_T15:
case MXT_TOUCH_PROXIMITY_T23:
case MXT_TOUCH_PROXKEY_T52:
+ case MXT_TOUCH_PTC_KEYS_T97:
case MXT_TOUCH_MULTITOUCHSCREEN_T100:
case MXT_PROCI_GRIPFACE_T20:
case MXT_PROCG_NOISE_T22:
@@ -891,6 +900,25 @@ static void mxt_proc_t9_message(struct mxt_data *data, u8 *message)
data->update_input = true;
}
+static void mxt_proc_t15_messages(struct mxt_data *data, u8 *message)
+{
+ struct input_dev *input_dev = data->input_dev;
+ unsigned long keystates = get_unaligned_le32(&message[2]);
+ int key;
+
+ for (key = 0; key < data->t15_num_keys; key++) {
+ input_report_key(input_dev, data->t15_keymap[key],
+ !!(keystates & BIT(key)));
+ }
+
+ data->update_input = true;
+}
+
+static void mxt_proc_t97_messages(struct mxt_data *data, u8 *message)
+{
+ mxt_proc_t15_messages(data, message);
+}
+
static void mxt_proc_t100_message(struct mxt_data *data, u8 *message)
{
struct device *dev = &data->client->dev;
@@ -1017,6 +1045,12 @@ static int mxt_proc_message(struct mxt_data *data, u8 *message)
} else if (report_id >= data->T9_reportid_min &&
report_id <= data->T9_reportid_max) {
mxt_proc_t9_message(data, message);
+ } else if (report_id >= data->T15_reportid_min &&
+ report_id <= data->T15_reportid_max) {
+ mxt_proc_t15_messages(data, message);
+ } else if (report_id >= data->T97_reportid_min &&
+ report_id <= data->T97_reportid_max) {
+ mxt_proc_t97_messages(data, message);
} else if (report_id >= data->T100_reportid_min &&
report_id <= data->T100_reportid_max) {
mxt_proc_t100_message(data, message);
@@ -1689,9 +1723,13 @@ static void mxt_free_object_table(struct mxt_data *data)
data->T71_address = 0;
data->T9_reportid_min = 0;
data->T9_reportid_max = 0;
+ data->T15_reportid_min = 0;
+ data->T15_reportid_max = 0;
data->T18_address = 0;
data->T19_reportid = 0;
data->T44_address = 0;
+ data->T97_reportid_min = 0;
+ data->T97_reportid_max = 0;
data->T100_reportid_min = 0;
data->T100_reportid_max = 0;
data->max_reportid = 0;
@@ -1764,6 +1802,10 @@ static int mxt_parse_object_table(struct mxt_data *data,
object->num_report_ids - 1;
data->num_touchids = object->num_report_ids;
break;
+ case MXT_TOUCH_KEYARRAY_T15:
+ data->T15_reportid_min = min_id;
+ data->T15_reportid_max = max_id;
+ break;
case MXT_SPT_COMMSCONFIG_T18:
data->T18_address = object->start_address;
break;
@@ -1773,6 +1815,10 @@ static int mxt_parse_object_table(struct mxt_data *data,
case MXT_SPT_GPIOPWM_T19:
data->T19_reportid = min_id;
break;
+ case MXT_TOUCH_PTC_KEYS_T97:
+ data->T97_reportid_min = min_id;
+ data->T97_reportid_max = max_id;
+ break;
case MXT_TOUCH_MULTITOUCHSCREEN_T100:
data->multitouch = MXT_TOUCH_MULTITOUCHSCREEN_T100;
data->T100_reportid_min = min_id;
@@ -2050,6 +2096,7 @@ static int mxt_initialize_input_device(struct mxt_data *data)
int error;
unsigned int num_mt_slots;
unsigned int mt_flags = 0;
+ int i;
switch (data->multitouch) {
case MXT_TOUCH_MULTI_T9:
@@ -2095,6 +2142,10 @@ static int mxt_initialize_input_device(struct mxt_data *data)
input_dev->open = mxt_input_open;
input_dev->close = mxt_input_close;
+ input_dev->keycode = data->t15_keymap;
+ input_dev->keycodemax = data->t15_num_keys;
+ input_dev->keycodesize = sizeof(data->t15_keymap[0]);
+
input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
/* For single touch */
@@ -2162,6 +2213,12 @@ static int mxt_initialize_input_device(struct mxt_data *data)
0, 255, 0, 0);
}
+ /* For T15 and T97 Key Array */
+ if (data->T15_reportid_min || data->T97_reportid_min) {
+ for (i = 0; i < data->t15_num_keys; i++)
+ input_set_capability(input_dev, EV_KEY, data->t15_keymap[i]);
+ }
+
input_set_drvdata(input_dev, data);
error = input_register_device(input_dev);
@@ -3080,8 +3137,10 @@ static void mxt_input_close(struct input_dev *dev)
static int mxt_parse_device_properties(struct mxt_data *data)
{
static const char keymap_property[] = "linux,gpio-keymap";
+ static const char buttons_property[] = "linux,keycodes";
struct device *dev = &data->client->dev;
u32 *keymap;
+ u32 *buttonmap;
int n_keys;
int error;
@@ -3111,6 +3170,32 @@ static int mxt_parse_device_properties(struct mxt_data *data)
data->t19_num_keys = n_keys;
}
+ if (device_property_present(dev, buttons_property)) {
+ n_keys = device_property_count_u32(dev, buttons_property);
+ if (n_keys <= 0) {
+ error = n_keys < 0 ? n_keys : -EINVAL;
+ dev_err(dev, "invalid/malformed '%s' property: %d\n",
+ buttons_property, error);
+ return error;
+ }
+
+ buttonmap = devm_kmalloc_array(dev, n_keys, sizeof(*buttonmap),
+ GFP_KERNEL);
+ if (!buttonmap)
+ return -ENOMEM;
+
+ error = device_property_read_u32_array(dev, buttons_property,
+ buttonmap, n_keys);
+ if (error) {
+ dev_err(dev, "failed to parse '%s' property: %d\n",
+ buttons_property, error);
+ return error;
+ }
+
+ data->t15_keymap = buttonmap;
+ data->t15_num_keys = n_keys;
+ }
+
return 0;
}
--
2.40.0
^ permalink raw reply related
* Re: [regression] Bug 216946 - Toshiba satellite click mini l9w-b: touchscreen: no touch events with kernel 6.1.4
From: Hans de Goede @ 2023-04-20 11:17 UTC (permalink / raw)
To: Gé Koerkamp, Linux regressions mailing list
Cc: Benjamin Tissoires, Jiri Kosina, Dmitry Torokhov,
open list:HID CORE LAYER, LKML
In-Reply-To: <07f2417e-9d47-3e73-cae5-3e7c95856f72@redhat.com>
Hi,
On 4/19/23 18:40, Hans de Goede wrote:
> Hi Gé,
>
> On 4/19/23 18:21, Gé Koerkamp wrote:
>> Hi Thorsten,
>>
>> travelling at the moment, but I’ll try. Not really into bisecting kernel issues, but let’s see where this goes. Another issue is, that I don’t have a Toshiba Mini Click myself, so I need to build the test Volumio images (volumio.org) with the different 5.16-rc kernel patch versions for a colleague to work with, hopefully he has time to do this.
>
> I have just dug my own Toshiba Mini Click up from the bottom of the storage box it was in.
>
> It is currently charging since the battery was fully flat.
>
> So I suggest you give me a couple of days to take a look before you go to all this trouble.
I have posted a patch fixing this upstream now:
https://lore.kernel.org/linux-acpi/20230420110220.23168-1-hdegoede@redhat.com/T/#u
Regards,
Hans
>>> On 17 Apr 2023, at 13:55, Linux regression tracking (Thorsten Leemhuis) <regressions@leemhuis.info> wrote:
>>>
>>>
>>>
>>>> On 01.03.23 12:41, Hans de Goede wrote:
>>>>> On 2/28/23 14:26, Benjamin Tissoires wrote:
>>>>> On Tue, Feb 28, 2023 at 12:32 PM Thorsten Leemhuis
>>>>> <regressions@leemhuis.info> wrote:
>>>>>>
>>>>>> On 19.01.23 16:06, Linux kernel regression tracking (Thorsten Leemhuis)
>>>>>> wrote:
>>>>>>> Hi, this is your Linux kernel regression tracker.
>>>>>>>
>>>>>>> I noticed a regression report in bugzilla.kernel.org. As many (most?)
>>>>>>> kernel developer don't keep an eye on it, I decided to forward it by
>>>>>>> mail. Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=216946 :
>>>>>>
>>>>>> The reporter recently confirmed in the ticket that the issue still
>>>>>> happens with 6.2.
>>>>>>
>>>>>> There wasn't any reply from any of the input developers here or in
>>>>>> bugzilla afaics. :-/ Hmmm. Could someone from that camp maybe please
>>>>>> take a minute and at least briefly look into this as answer something
>>>>>> like "that might be due to a problem in subsystem 'foo'", "maybe ask bar
>>>>>> for an option", or "we have no idea what might cause this, this needs to
>>>>>> be bisected"? That would help a lot.
>>>>>
>>>>> The working dmesg shows a line with:
>>>>> hid-generic 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00 Device
>>>>> [SIS0817:00 0457:10FB] on i2c-SIS0817:00
>>>>> and then
>>>>> hid-multitouch 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00
>>>>> Device [SIS0817:00 0457:10FB] on i2c-SIS0817:00
>>>>>
>>>>> But these 2 lines do not appear on the 6.1.4 logs.
>>>>>
>>>>> So the device is not properly enumerated by ACPI or I2C. Hans might
>>>>> have an idea on how to debug/solve that issue.
>>>>
>>>> I actually have a Toshiba satellite click mini l9w-b lying around
>>>> myself. I already made a note to try and reproduce this
>>>>
>>>> But I'm very much swamped with too much kernel work, so no promises
>>>> when I will get around to this ...
>>>
>>> Has anyone made any progress with this? Doesn't look like it, hence I
>>> wondered if this fall through the cracks; but maybe I'm just missing
>>> something.
>>>
>>> Gé Koerkamp: with a bit of luck Hans sooner or later will have time to
>>> look into this, but it might speed things up if you could try to bisect
>>> this.
>>>
>>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>> --
>>> Everything you wanna know about Linux kernel regression tracking:
>>> https://linux-regtracking.leemhuis.info/about/#tldr
>>> If I did something stupid, please tell me, as explained on that page.
>>>
>>> #regzbot poke
>>>
>>>>>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>>>>> --
>>>>>> Everything you wanna know about Linux kernel regression tracking:
>>>>>> https://linux-regtracking.leemhuis.info/about/#tldr
>>>>>> If I did something stupid, please tell me, as explained on that page.
>>>>>>
>>>>>> #regzbot poke
>>>>>>>> Gé Koerkamp 2023-01-17 20:21:51 UTC
>>>>>>>>
>>>>>>>> Created attachment 303619 [details]
>>>>>>>> Kernel configuration for v6.1.4/ journalctl (dmesg)/ ACPIdump/lsmod
>>>>>>>>
>>>>>>>> Overview:
>>>>>>>> The touchscreen does not react on touch events.
>>>>>>>> Touchscreen display and touchpad are working.
>>>>>>>>
>>>>>>>> Step to reproduce:
>>>>>>>> Open any UI page
>>>>>>>> Try to use touch on relevant UI controls (buttons etc.)
>>>>>>>>
>>>>>>>> Result:
>>>>>>>> No reaction on screen touches
>>>>>>>>
>>>>>>>> Expected result:
>>>>>>>> Reaction on touched control, same as when using the touch pad or connected mouse (which do work).
>>>>>>>>
>>>>>>>> Build information:
>>>>>>>> The error happens with kernel version 6.1.4
>>>>>>>> After rebuilding with different kernel versions, it appears that it first fails with kernel 5.16
>>>>>>>>
>>>>>>>> Additional builds:
>>>>>>>> The click mini l9w-b still works with kernel 5.10.y LTS and 5.15.y LTS.
>>>>>>>>
>>>>>>>> Important remark:
>>>>>>>> Touchscreen still works fine with kernel 6.1.4 using
>>>>>>>> - an HP x2 detachable 10-p0xx or
>>>>>>>> - a Lenovo yoga 520-14ikb
>>>>>>>>
>>>>>>>> So it could be a hardware dependent issue
>>>>>>>
>>>>>>> See the ticket for more details.
>>>>>>>
>>>>>>>
>>>>>>> [TLDR for the rest of this mail: I'm adding this report to the list of
>>>>>>> tracked Linux kernel regressions; the text you find below is based on a
>>>>>>> few templates paragraphs you might have encountered already in similar
>>>>>>> form.]
>>>>>>>
>>>>>>> BTW, let me use this mail to also add the report to the list of tracked
>>>>>>> regressions to ensure it's doesn't fall through the cracks:
>>>>>>>
>>>>>>> #regzbot introduced: v5.15..v5.16
>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=216946
>>>>>>> #regzbot title: hid: touchscreen broken with Toshiba satellite click
>>>>>>> mini l9w-b
>>>>>>> #regzbot ignore-activity
>>>>>>>
>>>>>>> This isn't a regression? This issue or a fix for it are already
>>>>>>> discussed somewhere else? It was fixed already? You want to clarify when
>>>>>>> the regression started to happen? Or point out I got the title or
>>>>>>> something else totally wrong? Then just reply and tell me -- ideally
>>>>>>> while also telling regzbot about it, as explained by the page listed in
>>>>>>> the footer of this mail.
>>>>>>>
>>>>>>> Developers: When fixing the issue, remember to add 'Link:' tags pointing
>>>>>>> to the report (e.g. the buzgzilla ticket and maybe this mail as well, if
>>>>>>> this thread sees some discussion). See page linked in footer for details.
>>>>>>>
>>>>>>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>>>>>> --
>>>>>>> Everything you wanna know about Linux kernel regression tracking:
>>>>>>> https://linux-regtracking.leemhuis.info/about/#tldr
>>>>>>> If I did something stupid, please tell me, as explained on that page.
>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>
>
^ permalink raw reply
* Re: [PATCH] Input: xpad - fix GPF in xpad_probe
From: Dan Carpenter @ 2023-04-20 11:07 UTC (permalink / raw)
To: Dan Carpenter
Cc: Dongliang Mu, Vicki Pfau, kernel-janitors, Dmitry Torokhov,
Pavel Rojtberg, Nate Yocom, Mattijs Korpershoek, John Butler,
Matthias Benkmann, Christopher Crockett, Santosh De Massari,
hust-os-kernel-patches, syzbot+a3f758b8d8cb7e49afec,
Pierre-Loup A. Griffais, linux-input, linux-kernel
In-Reply-To: <011c00e7-c351-4331-8be4-1c184d5773e1@kili.mountain>
[-- Attachment #1: Type: text/plain, Size: 2263 bytes --]
On Mon, Apr 17, 2023 at 01:42:21PM +0300, Dan Carpenter wrote:
> Btw, we should be thinking about how to detect these sorts of issues
> using static analysis. Unfortunately, it's not as simple as saying
> "We know this variable is NULL so don't dereference it." The problem
> with that is that many times Smatch sees where a pointer is set to NULL
> but not when it is assigned to a different value.
>
> What we could do instead is say:
> 1) If a pointer is dereferenced and we know it is NULL then:
> set_state_expr(my_id, expr, &suspicious);
> 2) If we set a pointer to non-NULL and it is marked as suspicious then
> print a warning.
I was thinking about this and it's not so simple. Normally after a
warning we return so the state never transitions from &suspicious to
non-NULL.
So what we could do is set the state to &suspicious. Then at the end of
the function we look at all the states at the return paths. If the
state is non-NULL on any return path then print a warning. This is easy
enough to do, but requires quite a bit of Smatch knowledge so I have
done it. Attached.
Unfortunately, it doesn't print a warning in this case because Smatch
doesn't track that _dev_warn() dereferences the "dev" pointer. The
__dev_printk() function only dereferences "dev" if it is non-NULL.
Smatch only counts it if it *always* dereferences it.
This could be fixed in two steps:
Step 1: track dereferences based on the return insead just yes/no.
Step 2: split _dev_warn() returns into two returns based on if dev is
NULL or non-NULL.
Step 1 is probably a good idea. Step 2 is a bad idea, because it makes
no sense to pass a NULL to dev_warn().
A better approach for this bug is to print a warning if people pass
the address of the offset from a NULL pointer. Combine that with the
same return states filter as earlier to eliminate false positives where
Smatch thinks a pointer is always NULL.
drivers/input/joystick/xpad.c:2053 xpad_probe() warn: address of NULL pointer 'xpad->dev'
drivers/media/i2c/ccs/ccs-data.c:524 ccs_data_parse_rules() warn: address of NULL pointer 'rules'
drivers/scsi/lpfc/lpfc_attr.c:1482 lpfc_reset_pci_bus() warn: address of NULL pointer 'phba->pcidev'
That check is attached too.
regards,
dan carpenter
[-- Attachment #2: check_deref_before_set.c --]
[-- Type: text/x-csrc, Size: 1714 bytes --]
/*
* Copyright 2023 Linaro Ltd.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, see http://www.gnu.org/copyleft/gpl.txt
*/
#include "smatch.h"
#include "smatch_extra.h"
#include "smatch_slist.h"
static int my_id;
STATE(suspicious);
static void deref_hook(struct expression *expr)
{
sval_t sval;
if (!get_implied_value(expr, &sval) || sval.value != 0)
return;
set_state_expr(my_id, expr, &suspicious);
}
static void check_variable(struct sm_state *sm)
{
struct sm_state *extra_sm, *tmp;
extra_sm = get_sm_state(SMATCH_EXTRA, sm->name, sm->sym);
if (!extra_sm)
return;
FOR_EACH_PTR(extra_sm->possible, tmp) {
if (!estate_rl(tmp->state))
continue;
if (rl_min(estate_rl(tmp->state)).value != 0) {
sm_warning_line(sm->line,
"pointer dereferenced without being set '%s'",
sm->name);
return;
}
} END_FOR_EACH_PTR(tmp);
}
static void process_states(void)
{
struct sm_state *tmp;
FOR_EACH_MY_SM(my_id, __get_cur_stree(), tmp) {
check_variable(tmp);
} END_FOR_EACH_SM(tmp);
}
void check_deref_before_set(int id)
{
my_id = id;
add_dereference_hook(deref_hook);
all_return_states_hook(&process_states);
}
[-- Attachment #3: check_bogus_address_param.c --]
[-- Type: text/x-csrc, Size: 2658 bytes --]
/*
* Copyright (C) 2019 Oracle.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, see http://www.gnu.org/copyleft/gpl.txt
*/
#include "smatch.h"
#include "smatch_extra.h"
#include "smatch_slist.h"
static int my_id;
STATE(suspicious);
static void check_param(struct expression *expr)
{
struct expression *tmp;
sval_t sval;
tmp = get_assigned_expr(expr);
if (tmp)
expr = tmp;
expr = strip_expr(expr);
if (!expr || expr->type != EXPR_PREOP || expr->op != '&')
return;
if (!get_implied_value(expr, &sval) ||
sval.value == 0 || sval.uvalue > 4096)
return;
expr = strip_expr(expr->unop);
while (expr->type == EXPR_DEREF) {
expr = strip_expr(expr->deref);
if (expr->type == EXPR_PREOP && expr->op == '*')
expr = strip_expr(expr->unop);
if (local_debug) {
struct range_list *rl;
get_absolute_rl(expr, &rl);
sm_msg("%s: expr='%s' type=%d rl='%s'", __func__, expr_to_str(expr), expr->type, show_rl(rl));
}
if (get_implied_value(expr, &sval) && sval.value == 0) {
set_state_expr(my_id, expr, &suspicious);
return;
}
}
}
static void match_call(struct expression *expr)
{
struct expression *arg;
FOR_EACH_PTR(expr->args, arg) {
check_param(arg);
} END_FOR_EACH_PTR(arg);
}
static void check_variable(struct sm_state *sm)
{
struct sm_state *extra_sm, *tmp;
int line = sm->line;
extra_sm = get_sm_state(SMATCH_EXTRA, sm->name, sm->sym);
if (!extra_sm)
return;
FOR_EACH_PTR(sm->possible, tmp) {
if (tmp->state == &suspicious)
line = tmp->line;
} END_FOR_EACH_PTR(tmp);
FOR_EACH_PTR(extra_sm->possible, tmp) {
if (!estate_rl(tmp->state))
continue;
if (rl_min(estate_rl(tmp->state)).value != 0) {
sm_warning_line(line, "address of NULL pointer '%s'",
sm->name);
return;
}
} END_FOR_EACH_PTR(tmp);
}
static void process_states(void)
{
struct sm_state *tmp;
FOR_EACH_MY_SM(my_id, __get_cur_stree(), tmp) {
check_variable(tmp);
} END_FOR_EACH_SM(tmp);
}
void check_bogus_address_param(int id)
{
my_id = id;
add_hook(&match_call, FUNCTION_CALL_HOOK);
all_return_states_hook(&process_states);
}
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: goodix: Add powered-in-suspend property
From: Fei Shao @ 2023-04-20 8:13 UTC (permalink / raw)
To: Doug Anderson
Cc: Jeff LaBundy, Benjamin Tissoires, Rob Herring, linux-mediatek,
Dmitry Torokhov, Krzysztof Kozlowski, devicetree, linux-input,
linux-kernel
In-Reply-To: <CAD=FV=UB393Z1zhK54Apgr-iRcvgiK0t36jt6-t5-5zz3m8OZQ@mail.gmail.com>
Hi,
On Wed, Apr 19, 2023 at 11:41 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Wed, Apr 19, 2023 at 8:18 AM Jeff LaBundy <jeff@labundy.com> wrote:
> >
> > Hi Doug and Fei,
> >
> > Thank you for the informative discussion; I can empathize with the pain
> > these issues bring.
> >
> > On Wed, Apr 19, 2023 at 07:38:13AM -0700, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Wed, Apr 19, 2023 at 3:44 AM Fei Shao <fshao@chromium.org> wrote:
> > > >
> > > > Hi Jeff,
> > > >
> > > > On Wed, Apr 19, 2023 at 8:21 AM Jeff LaBundy <jeff@labundy.com> wrote:
> > > > >
> > > > > Hi Fei,
> > > > >
> > > > > On Tue, Apr 18, 2023 at 08:49:51PM +0800, Fei Shao wrote:
> > > > > > We observed that on Chromebook device Steelix, if Goodix GT7375P
> > > > > > touchscreen is powered in suspend (because, for example, it connects to
> > > > > > an always-on regulator) and with the reset GPIO asserted, it will
> > > > > > introduce about 14mW power leakage.
> > > > > >
> > > > > > This property is used to indicate that the touchscreen is powered in
> > > > > > suspend. If it's set, the driver will stop asserting the reset GPIO in
> > > > > > power-down, and it will do it in power-up instead to ensure that the
> > > > > > state is always reset after resuming.
> > > > > >
> > > > > > Signed-off-by: Fei Shao <fshao@chromium.org>
> > > > > > ---
> > > > >
> > > > > This is an interesting problem; were you able to root-cause why the silicon
> > > > > exhibits this behavior? Simply asserting reset should not cause it to draw
> > > > > additional power, let alone 14 mW. This almost sounds like a back-powering
> > > > > problem during suspend.
> > > > >
> > > > There was a fix for this behavior before so I didn't dig into it on
> > > > the silicon side.
> > > > I can ask internally and see if we can have Goodix to confirm this is
> > > > a known HW erratum.
> > >
> > > Certainly it doesn't hurt to check, but it's not really that shocking
> > > to me that asserting reset could cause a power draw on some hardware.
> > > Reset puts hardware into a default state and that's not necessarily
> > > low power. I guess ideally hardware would act like it's "off" when
> > > reset is asserted and then then init to the default state on the edge
> > > as reset was deasserted, but I not all hardware is designed in an
> > > ideal way.
> >
> > While that is true in theory, I have never, ever seen that to be the case
> > when there is not some other underlying problem.
> >
> > What I have seen, however, is that asserting reset actually causes the GPIO
> > to sink current from some other supply and through the IC. I loosely suspect
> > that if you probe the IC's rails and digital I/O during the failure condition,
> > you may find one of them resting at some mid-rail voltage or diode drop. It
> > seems you have a similar suspicion.
> >
I reached out to our EE with your thoughts.
He said that he understands the concern, but this doesn't apply in our
schematics because there's only one supply.
Also he simulated a few scenarios that could explain the
back-powering, but none of them is possible without having the
problematic circuit/rsense layout from within the IC itself.
> > In that case, it may mean that some other supply in the system should actually
> > be kept on, or that supplies are being brought down out of order. In which
> > case, the solution should actually be a patch to the affected platform(s) dts
> > and not the mainline driver.
>
> I agree that it's a bandaid, but I'm not hopeful that a better
> solution will be found.
>
> Specifically, I'd expect a current leak in the system when you turn a
> supply off and then assert a GPIO high. That's when the device can
> start backpowering itself from a GPIO. In this case, it's the
> opposite. We're keeping the supply on and asserting the (active low)
> reset GPIO to cause the higher power draw. In another design it was
> confirmed that the power draw went away when we were able to turn the
> regulator off (but still keep the active low reset GPIO asserted).
> We've also confirmed that power is good if we keep the supply on and
> _don't_ assert the reset GPIO. Both of these two experiments provide
> some evidence that the system is configured properly and we're not
> backpowering something.
>
> I guess I should revise the above, though. I could believe that there
> is a current leak but on the touchscreen controller board itself,
> which is a black box to us. I have certainly been involved in products
> in the past where the default state of the system at reset caused a
> minor current leak (I remember an EE telling me that as soon as
> software started running I should quickly change the direction of a
> GPIO) and it wouldn't shock me if the touchscreen controller board had
> a problem like this. If there is a problem like this on the
> touchscreen controller board there's not much we can do to workaround
> it.
>
> Unfortunately, if the problem ends up needing a hardware change to fix
> more correctly (which I suspect it does), our hands are tied a bit.
> This is not prototype hardware but is final hardware.
>
> I guess one further note is that, at least on the project I was
> involved in that had a similar problem, folks in China did a bunch of
> analysis on this and went as far as adding an extra regulator to the
> main board schematic to "fix" it. Had the issue just been something
> where we were misconfiguing GPIOs or leaving a regulator in the wrong
> state then they (probably) would have identified it rather than
> spinning the board.
Thank you Doug for providing the details and explanation, and sorry
that I also missed your original reply...
I only considered the ideal scenarios for the always_on usage but not
the cases you brought up. The concerns make sense to me.
I'm still awaiting the response from Goodix, but +1 that if it turns
out to be a GT7375P hw issue, we're not able to do much about that
except relying on the driver workaround.
One more note I want to add is that the first attempt of the fix was
added ~2yrs ago, so it's not an one-off on a particular platform, plus
`sc7180-trogdor-homestar` and `mt8186-corsola-steelix` are two
different designs come from two different teams, but they ended up
with the same symptom.
With that said, I think we have more confidence to say it's a
component misbehavior, and we just fell into the edge case that was
not covered or considered by its design.
Regards,
Fei
>
> -Doug
^ permalink raw reply
* Re: [PATCH 2/2] input: imx_sc_key: add wakeup support
From: Peng Fan @ 2023-04-20 7:54 UTC (permalink / raw)
To: Ulf Hansson, Peng Fan
Cc: Dmitry Torokhov, robh+dt@kernel.org,
krzysztof.kozlowski+dt@linaro.org, shawnguo@kernel.org,
Aisheng Dong, linux-input@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com, dl-linux-imx
In-Reply-To: <CAPDyKFruZOP65k0SEEmCGtopp8ywJA92ChGZs2ZR=nVxqUC0OQ@mail.gmail.com>
Dmitry,Ulf
On 4/18/2023 4:32 PM, Ulf Hansson wrote:
> On Wed, 12 Apr 2023 at 17:58, Peng Fan <peng.fan@nxp.com> wrote:
>>
>> +Ulf
>>
>>> Subject: RE: [PATCH 2/2] input: imx_sc_key: add wakeup support
>>>
>>>> Subject: Re: [PATCH 2/2] input: imx_sc_key: add wakeup support
>>>>
>>>> On Thu, Mar 23, 2023 at 05:31:41PM +0800, Peng Fan (OSS) wrote:
>>>>> From: Peng Fan <peng.fan@nxp.com>
>>>>>
>>>>> Add support for waking up from system wide suspend.
>>>>>
>>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>>> ---
>>>>> drivers/input/keyboard/imx_sc_key.c | 2 ++
>>>>> 1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/drivers/input/keyboard/imx_sc_key.c
>>>> b/drivers/input/keyboard/imx_sc_key.c
>>>>> index d18839f1f4f6..234f23cf9990 100644
>>>>> --- a/drivers/input/keyboard/imx_sc_key.c
>>>>> +++ b/drivers/input/keyboard/imx_sc_key.c
>>>>> @@ -151,6 +151,8 @@ static int imx_sc_key_probe(struct
>>>> platform_device *pdev)
>>>>> priv->input = input;
>>>>> platform_set_drvdata(pdev, priv);
>>>>>
>>>>> + device_init_wakeup(&pdev->dev,
>>>> device_property_read_bool(&pdev->dev, "wakeup-source"));
>>>>> +
>>>>
>>>> I wonder - could we move this to the device core?
>>>
>>> I see lots device drivers parse wakeup-source, so I also follow That. Not sure
>>> whether could move this feature to device core, but anyway I could give a
>>> try.
>>
>> Do you think it is feasible to move device_init_wakeup into device core
>> part?
>
> Not sure it would really improve things that much. Subsystems/drivers
> need to make additional configurations based upon whether this DT
> property is set anyway.
>
> Perhaps an option is to make this a part of the common input subsystem
> helper functions instead? Other subsystems do this, but I am not sure
> how feasible that would be in the input case.
How do you think of below patch?
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 37e876d45eb9..a98a9f37e1f5 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -2402,6 +2402,10 @@ int input_register_device(struct input_dev *dev)
__func__, dev_name(&dev->dev));
devres_add(dev->dev.parent, devres);
}
+
+ if (device_property_read_bool(input->dev.parent, "wakeup-source"))
+ device_init_wakeup(&pdev->dev, true);
+
return 0;
err_device_del:
Thanks,
Peng.
>
> Kind regards
> Uffe
^ permalink raw reply related
* Re: [PATCH] dt-bindings: input: touchscreen: goodix: clarify irq-gpios misleading text
From: Jeff LaBundy @ 2023-04-19 19:15 UTC (permalink / raw)
To: Luca Ceresoli
Cc: Dmitry Torokhov, devicetree, Rob Herring, Krzysztof Kozlowski,
Stephen Boyd, Jarrah Gosbell, linux-input, linux-kernel,
Thomas Petazzoni
In-Reply-To: <20230419171849.946986-1-luca.ceresoli@bootlin.com>
Hi Luca,
On Wed, Apr 19, 2023 at 07:18:49PM +0200, Luca Ceresoli wrote:
> The irq-gpios description misleading, apparently saying that driving the
> IRQ GPIO resets the device, which is even more puzzling as there is a reset
> GPIO as well.
>
> In reality the IRQ pin can be driven during the reset sequence to configure
> the client address, as it becomes clear after checking both the datasheet
> and the driver code. Improve the text to clarify that.
>
> Also rephrase to remove reference to the driver, which is not appropriate
> in the bindings.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
> .../devicetree/bindings/input/touchscreen/goodix.yaml | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.yaml b/Documentation/devicetree/bindings/input/touchscreen/goodix.yaml
> index 3d016b87c8df..26ad104052ba 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.yaml
> @@ -37,8 +37,8 @@ properties:
> maxItems: 1
>
> irq-gpios:
> - description: GPIO pin used for IRQ. The driver uses the interrupt gpio pin
> - as output to reset the device.
> + description: GPIO pin used for IRQ. The IRQ line can be driven during the
> + reset sequence to configure the client address.
This is a step in the right direction, but in my humble opinion it is now
ambiguous who drives the bidirectional IRQ pin (device or driver). Perhaps
combining both versions conveys the appropriate message:
"...The driver uses the interrupt GPIO pin as an output to configure the client
address during the reset sequence."
> maxItems: 1
>
> reset-gpios:
> --
> 2.34.1
>
Kind regards,
Jeff LaBundy
^ permalink raw reply
* [PATCH] dt-bindings: input: touchscreen: goodix: clarify irq-gpios misleading text
From: Luca Ceresoli @ 2023-04-19 17:18 UTC (permalink / raw)
To: Dmitry Torokhov, devicetree
Cc: Luca Ceresoli, Rob Herring, Krzysztof Kozlowski, Stephen Boyd,
Jarrah Gosbell, linux-input, linux-kernel, Thomas Petazzoni
The irq-gpios description misleading, apparently saying that driving the
IRQ GPIO resets the device, which is even more puzzling as there is a reset
GPIO as well.
In reality the IRQ pin can be driven during the reset sequence to configure
the client address, as it becomes clear after checking both the datasheet
and the driver code. Improve the text to clarify that.
Also rephrase to remove reference to the driver, which is not appropriate
in the bindings.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
.../devicetree/bindings/input/touchscreen/goodix.yaml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.yaml b/Documentation/devicetree/bindings/input/touchscreen/goodix.yaml
index 3d016b87c8df..26ad104052ba 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/goodix.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.yaml
@@ -37,8 +37,8 @@ properties:
maxItems: 1
irq-gpios:
- description: GPIO pin used for IRQ. The driver uses the interrupt gpio pin
- as output to reset the device.
+ description: GPIO pin used for IRQ. The IRQ line can be driven during the
+ reset sequence to configure the client address.
maxItems: 1
reset-gpios:
--
2.34.1
^ permalink raw reply related
* Re: [regression] Bug 216946 - Toshiba satellite click mini l9w-b: touchscreen: no touch events with kernel 6.1.4
From: Hans de Goede @ 2023-04-19 16:40 UTC (permalink / raw)
To: Gé Koerkamp, Linux regressions mailing list
Cc: Benjamin Tissoires, Jiri Kosina, Dmitry Torokhov,
open list:HID CORE LAYER, LKML
In-Reply-To: <E4123C4B-D6C0-4CB2-8328-E0D03599ED16@gmail.com>
Hi Gé,
On 4/19/23 18:21, Gé Koerkamp wrote:
> Hi Thorsten,
>
> travelling at the moment, but I’ll try. Not really into bisecting kernel issues, but let’s see where this goes. Another issue is, that I don’t have a Toshiba Mini Click myself, so I need to build the test Volumio images (volumio.org) with the different 5.16-rc kernel patch versions for a colleague to work with, hopefully he has time to do this.
I have just dug my own Toshiba Mini Click up from the bottom of the storage box it was in.
It is currently charging since the battery was fully flat.
So I suggest you give me a couple of days to take a look before you go to all this trouble.
Regards,
Hans
>
> Thanks - Gé
>
> Sent from my iPhone
>
>> On 17 Apr 2023, at 13:55, Linux regression tracking (Thorsten Leemhuis) <regressions@leemhuis.info> wrote:
>>
>>
>>
>>> On 01.03.23 12:41, Hans de Goede wrote:
>>>> On 2/28/23 14:26, Benjamin Tissoires wrote:
>>>> On Tue, Feb 28, 2023 at 12:32 PM Thorsten Leemhuis
>>>> <regressions@leemhuis.info> wrote:
>>>>>
>>>>> On 19.01.23 16:06, Linux kernel regression tracking (Thorsten Leemhuis)
>>>>> wrote:
>>>>>> Hi, this is your Linux kernel regression tracker.
>>>>>>
>>>>>> I noticed a regression report in bugzilla.kernel.org. As many (most?)
>>>>>> kernel developer don't keep an eye on it, I decided to forward it by
>>>>>> mail. Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=216946 :
>>>>>
>>>>> The reporter recently confirmed in the ticket that the issue still
>>>>> happens with 6.2.
>>>>>
>>>>> There wasn't any reply from any of the input developers here or in
>>>>> bugzilla afaics. :-/ Hmmm. Could someone from that camp maybe please
>>>>> take a minute and at least briefly look into this as answer something
>>>>> like "that might be due to a problem in subsystem 'foo'", "maybe ask bar
>>>>> for an option", or "we have no idea what might cause this, this needs to
>>>>> be bisected"? That would help a lot.
>>>>
>>>> The working dmesg shows a line with:
>>>> hid-generic 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00 Device
>>>> [SIS0817:00 0457:10FB] on i2c-SIS0817:00
>>>> and then
>>>> hid-multitouch 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00
>>>> Device [SIS0817:00 0457:10FB] on i2c-SIS0817:00
>>>>
>>>> But these 2 lines do not appear on the 6.1.4 logs.
>>>>
>>>> So the device is not properly enumerated by ACPI or I2C. Hans might
>>>> have an idea on how to debug/solve that issue.
>>>
>>> I actually have a Toshiba satellite click mini l9w-b lying around
>>> myself. I already made a note to try and reproduce this
>>>
>>> But I'm very much swamped with too much kernel work, so no promises
>>> when I will get around to this ...
>>
>> Has anyone made any progress with this? Doesn't look like it, hence I
>> wondered if this fall through the cracks; but maybe I'm just missing
>> something.
>>
>> Gé Koerkamp: with a bit of luck Hans sooner or later will have time to
>> look into this, but it might speed things up if you could try to bisect
>> this.
>>
>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>> --
>> Everything you wanna know about Linux kernel regression tracking:
>> https://linux-regtracking.leemhuis.info/about/#tldr
>> If I did something stupid, please tell me, as explained on that page.
>>
>> #regzbot poke
>>
>>>>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>>>> --
>>>>> Everything you wanna know about Linux kernel regression tracking:
>>>>> https://linux-regtracking.leemhuis.info/about/#tldr
>>>>> If I did something stupid, please tell me, as explained on that page.
>>>>>
>>>>> #regzbot poke
>>>>>>> Gé Koerkamp 2023-01-17 20:21:51 UTC
>>>>>>>
>>>>>>> Created attachment 303619 [details]
>>>>>>> Kernel configuration for v6.1.4/ journalctl (dmesg)/ ACPIdump/lsmod
>>>>>>>
>>>>>>> Overview:
>>>>>>> The touchscreen does not react on touch events.
>>>>>>> Touchscreen display and touchpad are working.
>>>>>>>
>>>>>>> Step to reproduce:
>>>>>>> Open any UI page
>>>>>>> Try to use touch on relevant UI controls (buttons etc.)
>>>>>>>
>>>>>>> Result:
>>>>>>> No reaction on screen touches
>>>>>>>
>>>>>>> Expected result:
>>>>>>> Reaction on touched control, same as when using the touch pad or connected mouse (which do work).
>>>>>>>
>>>>>>> Build information:
>>>>>>> The error happens with kernel version 6.1.4
>>>>>>> After rebuilding with different kernel versions, it appears that it first fails with kernel 5.16
>>>>>>>
>>>>>>> Additional builds:
>>>>>>> The click mini l9w-b still works with kernel 5.10.y LTS and 5.15.y LTS.
>>>>>>>
>>>>>>> Important remark:
>>>>>>> Touchscreen still works fine with kernel 6.1.4 using
>>>>>>> - an HP x2 detachable 10-p0xx or
>>>>>>> - a Lenovo yoga 520-14ikb
>>>>>>>
>>>>>>> So it could be a hardware dependent issue
>>>>>>
>>>>>> See the ticket for more details.
>>>>>>
>>>>>>
>>>>>> [TLDR for the rest of this mail: I'm adding this report to the list of
>>>>>> tracked Linux kernel regressions; the text you find below is based on a
>>>>>> few templates paragraphs you might have encountered already in similar
>>>>>> form.]
>>>>>>
>>>>>> BTW, let me use this mail to also add the report to the list of tracked
>>>>>> regressions to ensure it's doesn't fall through the cracks:
>>>>>>
>>>>>> #regzbot introduced: v5.15..v5.16
>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=216946
>>>>>> #regzbot title: hid: touchscreen broken with Toshiba satellite click
>>>>>> mini l9w-b
>>>>>> #regzbot ignore-activity
>>>>>>
>>>>>> This isn't a regression? This issue or a fix for it are already
>>>>>> discussed somewhere else? It was fixed already? You want to clarify when
>>>>>> the regression started to happen? Or point out I got the title or
>>>>>> something else totally wrong? Then just reply and tell me -- ideally
>>>>>> while also telling regzbot about it, as explained by the page listed in
>>>>>> the footer of this mail.
>>>>>>
>>>>>> Developers: When fixing the issue, remember to add 'Link:' tags pointing
>>>>>> to the report (e.g. the buzgzilla ticket and maybe this mail as well, if
>>>>>> this thread sees some discussion). See page linked in footer for details.
>>>>>>
>>>>>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>>>>> --
>>>>>> Everything you wanna know about Linux kernel regression tracking:
>>>>>> https://linux-regtracking.leemhuis.info/about/#tldr
>>>>>> If I did something stupid, please tell me, as explained on that page.
>>>>>
>>>>
>>>
>>>
>>>
>
^ permalink raw reply
* Re: [regression] Bug 216946 - Toshiba satellite click mini l9w-b: touchscreen: no touch events with kernel 6.1.4
From: Gé Koerkamp @ 2023-04-19 16:21 UTC (permalink / raw)
To: Linux regressions mailing list
Cc: Hans de Goede, Benjamin Tissoires, Jiri Kosina, Dmitry Torokhov,
open list:HID CORE LAYER, LKML
In-Reply-To: <993dd537-2f74-3657-1dd3-c3a0a74e37e1@leemhuis.info>
Hi Thorsten,
travelling at the moment, but I’ll try. Not really into bisecting kernel issues, but let’s see where this goes. Another issue is, that I don’t have a Toshiba Mini Click myself, so I need to build the test Volumio images (volumio.org) with the different 5.16-rc kernel patch versions for a colleague to work with, hopefully he has time to do this.
Thanks - Gé
Sent from my iPhone
> On 17 Apr 2023, at 13:55, Linux regression tracking (Thorsten Leemhuis) <regressions@leemhuis.info> wrote:
>
>
>
>> On 01.03.23 12:41, Hans de Goede wrote:
>>> On 2/28/23 14:26, Benjamin Tissoires wrote:
>>> On Tue, Feb 28, 2023 at 12:32 PM Thorsten Leemhuis
>>> <regressions@leemhuis.info> wrote:
>>>>
>>>> On 19.01.23 16:06, Linux kernel regression tracking (Thorsten Leemhuis)
>>>> wrote:
>>>>> Hi, this is your Linux kernel regression tracker.
>>>>>
>>>>> I noticed a regression report in bugzilla.kernel.org. As many (most?)
>>>>> kernel developer don't keep an eye on it, I decided to forward it by
>>>>> mail. Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=216946 :
>>>>
>>>> The reporter recently confirmed in the ticket that the issue still
>>>> happens with 6.2.
>>>>
>>>> There wasn't any reply from any of the input developers here or in
>>>> bugzilla afaics. :-/ Hmmm. Could someone from that camp maybe please
>>>> take a minute and at least briefly look into this as answer something
>>>> like "that might be due to a problem in subsystem 'foo'", "maybe ask bar
>>>> for an option", or "we have no idea what might cause this, this needs to
>>>> be bisected"? That would help a lot.
>>>
>>> The working dmesg shows a line with:
>>> hid-generic 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00 Device
>>> [SIS0817:00 0457:10FB] on i2c-SIS0817:00
>>> and then
>>> hid-multitouch 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00
>>> Device [SIS0817:00 0457:10FB] on i2c-SIS0817:00
>>>
>>> But these 2 lines do not appear on the 6.1.4 logs.
>>>
>>> So the device is not properly enumerated by ACPI or I2C. Hans might
>>> have an idea on how to debug/solve that issue.
>>
>> I actually have a Toshiba satellite click mini l9w-b lying around
>> myself. I already made a note to try and reproduce this
>>
>> But I'm very much swamped with too much kernel work, so no promises
>> when I will get around to this ...
>
> Has anyone made any progress with this? Doesn't look like it, hence I
> wondered if this fall through the cracks; but maybe I'm just missing
> something.
>
> Gé Koerkamp: with a bit of luck Hans sooner or later will have time to
> look into this, but it might speed things up if you could try to bisect
> this.
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> #regzbot poke
>
>>>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>>> --
>>>> Everything you wanna know about Linux kernel regression tracking:
>>>> https://linux-regtracking.leemhuis.info/about/#tldr
>>>> If I did something stupid, please tell me, as explained on that page.
>>>>
>>>> #regzbot poke
>>>>>> Gé Koerkamp 2023-01-17 20:21:51 UTC
>>>>>>
>>>>>> Created attachment 303619 [details]
>>>>>> Kernel configuration for v6.1.4/ journalctl (dmesg)/ ACPIdump/lsmod
>>>>>>
>>>>>> Overview:
>>>>>> The touchscreen does not react on touch events.
>>>>>> Touchscreen display and touchpad are working.
>>>>>>
>>>>>> Step to reproduce:
>>>>>> Open any UI page
>>>>>> Try to use touch on relevant UI controls (buttons etc.)
>>>>>>
>>>>>> Result:
>>>>>> No reaction on screen touches
>>>>>>
>>>>>> Expected result:
>>>>>> Reaction on touched control, same as when using the touch pad or connected mouse (which do work).
>>>>>>
>>>>>> Build information:
>>>>>> The error happens with kernel version 6.1.4
>>>>>> After rebuilding with different kernel versions, it appears that it first fails with kernel 5.16
>>>>>>
>>>>>> Additional builds:
>>>>>> The click mini l9w-b still works with kernel 5.10.y LTS and 5.15.y LTS.
>>>>>>
>>>>>> Important remark:
>>>>>> Touchscreen still works fine with kernel 6.1.4 using
>>>>>> - an HP x2 detachable 10-p0xx or
>>>>>> - a Lenovo yoga 520-14ikb
>>>>>>
>>>>>> So it could be a hardware dependent issue
>>>>>
>>>>> See the ticket for more details.
>>>>>
>>>>>
>>>>> [TLDR for the rest of this mail: I'm adding this report to the list of
>>>>> tracked Linux kernel regressions; the text you find below is based on a
>>>>> few templates paragraphs you might have encountered already in similar
>>>>> form.]
>>>>>
>>>>> BTW, let me use this mail to also add the report to the list of tracked
>>>>> regressions to ensure it's doesn't fall through the cracks:
>>>>>
>>>>> #regzbot introduced: v5.15..v5.16
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=216946
>>>>> #regzbot title: hid: touchscreen broken with Toshiba satellite click
>>>>> mini l9w-b
>>>>> #regzbot ignore-activity
>>>>>
>>>>> This isn't a regression? This issue or a fix for it are already
>>>>> discussed somewhere else? It was fixed already? You want to clarify when
>>>>> the regression started to happen? Or point out I got the title or
>>>>> something else totally wrong? Then just reply and tell me -- ideally
>>>>> while also telling regzbot about it, as explained by the page listed in
>>>>> the footer of this mail.
>>>>>
>>>>> Developers: When fixing the issue, remember to add 'Link:' tags pointing
>>>>> to the report (e.g. the buzgzilla ticket and maybe this mail as well, if
>>>>> this thread sees some discussion). See page linked in footer for details.
>>>>>
>>>>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>>>> --
>>>>> Everything you wanna know about Linux kernel regression tracking:
>>>>> https://linux-regtracking.leemhuis.info/about/#tldr
>>>>> If I did something stupid, please tell me, as explained on that page.
>>>>
>>>
>>
>>
>>
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: goodix: Add powered-in-suspend property
From: Doug Anderson @ 2023-04-19 15:41 UTC (permalink / raw)
To: Jeff LaBundy
Cc: Fei Shao, Benjamin Tissoires, Rob Herring, linux-mediatek,
Dmitry Torokhov, Krzysztof Kozlowski, devicetree, linux-input,
linux-kernel
In-Reply-To: <ZEAGTiGyvynGA9P1@nixie71>
Hi,
On Wed, Apr 19, 2023 at 8:18 AM Jeff LaBundy <jeff@labundy.com> wrote:
>
> Hi Doug and Fei,
>
> Thank you for the informative discussion; I can empathize with the pain
> these issues bring.
>
> On Wed, Apr 19, 2023 at 07:38:13AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Wed, Apr 19, 2023 at 3:44 AM Fei Shao <fshao@chromium.org> wrote:
> > >
> > > Hi Jeff,
> > >
> > > On Wed, Apr 19, 2023 at 8:21 AM Jeff LaBundy <jeff@labundy.com> wrote:
> > > >
> > > > Hi Fei,
> > > >
> > > > On Tue, Apr 18, 2023 at 08:49:51PM +0800, Fei Shao wrote:
> > > > > We observed that on Chromebook device Steelix, if Goodix GT7375P
> > > > > touchscreen is powered in suspend (because, for example, it connects to
> > > > > an always-on regulator) and with the reset GPIO asserted, it will
> > > > > introduce about 14mW power leakage.
> > > > >
> > > > > This property is used to indicate that the touchscreen is powered in
> > > > > suspend. If it's set, the driver will stop asserting the reset GPIO in
> > > > > power-down, and it will do it in power-up instead to ensure that the
> > > > > state is always reset after resuming.
> > > > >
> > > > > Signed-off-by: Fei Shao <fshao@chromium.org>
> > > > > ---
> > > >
> > > > This is an interesting problem; were you able to root-cause why the silicon
> > > > exhibits this behavior? Simply asserting reset should not cause it to draw
> > > > additional power, let alone 14 mW. This almost sounds like a back-powering
> > > > problem during suspend.
> > > >
> > > There was a fix for this behavior before so I didn't dig into it on
> > > the silicon side.
> > > I can ask internally and see if we can have Goodix to confirm this is
> > > a known HW erratum.
> >
> > Certainly it doesn't hurt to check, but it's not really that shocking
> > to me that asserting reset could cause a power draw on some hardware.
> > Reset puts hardware into a default state and that's not necessarily
> > low power. I guess ideally hardware would act like it's "off" when
> > reset is asserted and then then init to the default state on the edge
> > as reset was deasserted, but I not all hardware is designed in an
> > ideal way.
>
> While that is true in theory, I have never, ever seen that to be the case
> when there is not some other underlying problem.
>
> What I have seen, however, is that asserting reset actually causes the GPIO
> to sink current from some other supply and through the IC. I loosely suspect
> that if you probe the IC's rails and digital I/O during the failure condition,
> you may find one of them resting at some mid-rail voltage or diode drop. It
> seems you have a similar suspicion.
>
> In that case, it may mean that some other supply in the system should actually
> be kept on, or that supplies are being brought down out of order. In which
> case, the solution should actually be a patch to the affected platform(s) dts
> and not the mainline driver.
I agree that it's a bandaid, but I'm not hopeful that a better
solution will be found.
Specifically, I'd expect a current leak in the system when you turn a
supply off and then assert a GPIO high. That's when the device can
start backpowering itself from a GPIO. In this case, it's the
opposite. We're keeping the supply on and asserting the (active low)
reset GPIO to cause the higher power draw. In another design it was
confirmed that the power draw went away when we were able to turn the
regulator off (but still keep the active low reset GPIO asserted).
We've also confirmed that power is good if we keep the supply on and
_don't_ assert the reset GPIO. Both of these two experiments provide
some evidence that the system is configured properly and we're not
backpowering something.
I guess I should revise the above, though. I could believe that there
is a current leak but on the touchscreen controller board itself,
which is a black box to us. I have certainly been involved in products
in the past where the default state of the system at reset caused a
minor current leak (I remember an EE telling me that as soon as
software started running I should quickly change the direction of a
GPIO) and it wouldn't shock me if the touchscreen controller board had
a problem like this. If there is a problem like this on the
touchscreen controller board there's not much we can do to workaround
it.
Unfortunately, if the problem ends up needing a hardware change to fix
more correctly (which I suspect it does), our hands are tied a bit.
This is not prototype hardware but is final hardware.
I guess one further note is that, at least on the project I was
involved in that had a similar problem, folks in China did a bunch of
analysis on this and went as far as adding an extra regulator to the
main board schematic to "fix" it. Had the issue just been something
where we were misconfiguing GPIOs or leaving a regulator in the wrong
state then they (probably) would have identified it rather than
spinning the board.
-Doug
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: goodix: Add powered-in-suspend property
From: Jeff LaBundy @ 2023-04-19 15:18 UTC (permalink / raw)
To: Doug Anderson
Cc: Fei Shao, Benjamin Tissoires, Rob Herring, linux-mediatek,
Dmitry Torokhov, Krzysztof Kozlowski, devicetree, linux-input,
linux-kernel
In-Reply-To: <CAD=FV=U_i26a8uJYmqYf6PUgmTUgmEB5L2DkVga0zDX_iDcGQg@mail.gmail.com>
Hi Doug and Fei,
Thank you for the informative discussion; I can empathize with the pain
these issues bring.
On Wed, Apr 19, 2023 at 07:38:13AM -0700, Doug Anderson wrote:
> Hi,
>
> On Wed, Apr 19, 2023 at 3:44 AM Fei Shao <fshao@chromium.org> wrote:
> >
> > Hi Jeff,
> >
> > On Wed, Apr 19, 2023 at 8:21 AM Jeff LaBundy <jeff@labundy.com> wrote:
> > >
> > > Hi Fei,
> > >
> > > On Tue, Apr 18, 2023 at 08:49:51PM +0800, Fei Shao wrote:
> > > > We observed that on Chromebook device Steelix, if Goodix GT7375P
> > > > touchscreen is powered in suspend (because, for example, it connects to
> > > > an always-on regulator) and with the reset GPIO asserted, it will
> > > > introduce about 14mW power leakage.
> > > >
> > > > This property is used to indicate that the touchscreen is powered in
> > > > suspend. If it's set, the driver will stop asserting the reset GPIO in
> > > > power-down, and it will do it in power-up instead to ensure that the
> > > > state is always reset after resuming.
> > > >
> > > > Signed-off-by: Fei Shao <fshao@chromium.org>
> > > > ---
> > >
> > > This is an interesting problem; were you able to root-cause why the silicon
> > > exhibits this behavior? Simply asserting reset should not cause it to draw
> > > additional power, let alone 14 mW. This almost sounds like a back-powering
> > > problem during suspend.
> > >
> > There was a fix for this behavior before so I didn't dig into it on
> > the silicon side.
> > I can ask internally and see if we can have Goodix to confirm this is
> > a known HW erratum.
>
> Certainly it doesn't hurt to check, but it's not really that shocking
> to me that asserting reset could cause a power draw on some hardware.
> Reset puts hardware into a default state and that's not necessarily
> low power. I guess ideally hardware would act like it's "off" when
> reset is asserted and then then init to the default state on the edge
> as reset was deasserted, but I not all hardware is designed in an
> ideal way.
While that is true in theory, I have never, ever seen that to be the case
when there is not some other underlying problem.
What I have seen, however, is that asserting reset actually causes the GPIO
to sink current from some other supply and through the IC. I loosely suspect
that if you probe the IC's rails and digital I/O during the failure condition,
you may find one of them resting at some mid-rail voltage or diode drop. It
seems you have a similar suspicion.
In that case, it may mean that some other supply in the system should actually
be kept on, or that supplies are being brought down out of order. In which
case, the solution should actually be a patch to the affected platform(s) dts
and not the mainline driver.
>
>
> > > If this is truly expected behavior, is it sufficient to use the always_on
> > > constraint of the relevant regulator(s) to make this decision as opposed to
> > > introducing a new property?
> > >
> > That sounds good to me. IIUC, for the existing designs, the boards
> > that would set this property would also exclusively set
> > `regulator-always-on` in their supply, so that should suffice.
> > Let me revise the patch. Thanks!
>
> Yeah, I thought about this too and talked about it in my original
> reply. It doesn't handle the shared-rail case, but then again neither
> does ${SUBJECT} patch. ...then I guess the only argument against it is
> my argument that the regulator could be marked "always-on" in the
> device tree but still turned off by an external entity (PMIC or EC) in
> S3. In theory this should be specified by
> "regulator-state-(standby|mem|disk)", but I could believe it being
> tricky to figure out (what if a parent regulator gets turned off
> automatically but the child isn't explicit?). Specifically, if a
> regulator is always-on but somehow gets shut off in suspend then we
> _do_ want to assert reset (active low) during suspend, otherwise we'll
> have a power leak through the reset GPIO... :-P
D'oh! Sorry I missed your original reply. My concern is that either solution
is a band-aid and does not address the root cause. I would rather see a patch
that addresses what seems to be a back-powering problem so that the driver may
freely assert reset. That is just my $.02; let me know if I have misunderstood
or there are other factors that prevent that path from being viable.
>
> ...so I guess I'll continue to assert that I don't think peeking at
> the regulator's "always-on" property is the best way to go. If
> everyone else disagrees with me then I won't stand in the way, but IMO
> the extra property like Fei's patch adds is better.
>
> [1] https://lore.kernel.org/r/CAD=FV=V8ZN3969RrPu2-zZYoEE=LDxpi8K_E8EziiDpGOSsq1w@mail.gmail.com
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: goodix: Add powered-in-suspend property
From: Doug Anderson @ 2023-04-19 14:38 UTC (permalink / raw)
To: Fei Shao
Cc: Jeff LaBundy, Benjamin Tissoires, Rob Herring, linux-mediatek,
Dmitry Torokhov, Krzysztof Kozlowski, devicetree, linux-input,
linux-kernel
In-Reply-To: <CAC=S1ngBt9DmBobMkQXWhqE1UUxFv2U6iFd42nT=1N7r8+pFUg@mail.gmail.com>
Hi,
On Wed, Apr 19, 2023 at 3:44 AM Fei Shao <fshao@chromium.org> wrote:
>
> Hi Jeff,
>
> On Wed, Apr 19, 2023 at 8:21 AM Jeff LaBundy <jeff@labundy.com> wrote:
> >
> > Hi Fei,
> >
> > On Tue, Apr 18, 2023 at 08:49:51PM +0800, Fei Shao wrote:
> > > We observed that on Chromebook device Steelix, if Goodix GT7375P
> > > touchscreen is powered in suspend (because, for example, it connects to
> > > an always-on regulator) and with the reset GPIO asserted, it will
> > > introduce about 14mW power leakage.
> > >
> > > This property is used to indicate that the touchscreen is powered in
> > > suspend. If it's set, the driver will stop asserting the reset GPIO in
> > > power-down, and it will do it in power-up instead to ensure that the
> > > state is always reset after resuming.
> > >
> > > Signed-off-by: Fei Shao <fshao@chromium.org>
> > > ---
> >
> > This is an interesting problem; were you able to root-cause why the silicon
> > exhibits this behavior? Simply asserting reset should not cause it to draw
> > additional power, let alone 14 mW. This almost sounds like a back-powering
> > problem during suspend.
> >
> There was a fix for this behavior before so I didn't dig into it on
> the silicon side.
> I can ask internally and see if we can have Goodix to confirm this is
> a known HW erratum.
Certainly it doesn't hurt to check, but it's not really that shocking
to me that asserting reset could cause a power draw on some hardware.
Reset puts hardware into a default state and that's not necessarily
low power. I guess ideally hardware would act like it's "off" when
reset is asserted and then then init to the default state on the edge
as reset was deasserted, but I not all hardware is designed in an
ideal way.
> > If this is truly expected behavior, is it sufficient to use the always_on
> > constraint of the relevant regulator(s) to make this decision as opposed to
> > introducing a new property?
> >
> That sounds good to me. IIUC, for the existing designs, the boards
> that would set this property would also exclusively set
> `regulator-always-on` in their supply, so that should suffice.
> Let me revise the patch. Thanks!
Yeah, I thought about this too and talked about it in my original
reply. It doesn't handle the shared-rail case, but then again neither
does ${SUBJECT} patch. ...then I guess the only argument against it is
my argument that the regulator could be marked "always-on" in the
device tree but still turned off by an external entity (PMIC or EC) in
S3. In theory this should be specified by
"regulator-state-(standby|mem|disk)", but I could believe it being
tricky to figure out (what if a parent regulator gets turned off
automatically but the child isn't explicit?). Specifically, if a
regulator is always-on but somehow gets shut off in suspend then we
_do_ want to assert reset (active low) during suspend, otherwise we'll
have a power leak through the reset GPIO... :-P
...so I guess I'll continue to assert that I don't think peeking at
the regulator's "always-on" property is the best way to go. If
everyone else disagrees with me then I won't stand in the way, but IMO
the extra property like Fei's patch adds is better.
[1] https://lore.kernel.org/r/CAD=FV=V8ZN3969RrPu2-zZYoEE=LDxpi8K_E8EziiDpGOSsq1w@mail.gmail.com
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: goodix: Add powered-in-suspend property
From: Fei Shao @ 2023-04-19 10:44 UTC (permalink / raw)
To: Jeff LaBundy
Cc: Benjamin Tissoires, Douglas Anderson, Rob Herring, linux-mediatek,
Dmitry Torokhov, Krzysztof Kozlowski, devicetree, linux-input,
linux-kernel
In-Reply-To: <ZD8z57MBvcfExJx8@nixie71>
Hi Jeff,
On Wed, Apr 19, 2023 at 8:21 AM Jeff LaBundy <jeff@labundy.com> wrote:
>
> Hi Fei,
>
> On Tue, Apr 18, 2023 at 08:49:51PM +0800, Fei Shao wrote:
> > We observed that on Chromebook device Steelix, if Goodix GT7375P
> > touchscreen is powered in suspend (because, for example, it connects to
> > an always-on regulator) and with the reset GPIO asserted, it will
> > introduce about 14mW power leakage.
> >
> > This property is used to indicate that the touchscreen is powered in
> > suspend. If it's set, the driver will stop asserting the reset GPIO in
> > power-down, and it will do it in power-up instead to ensure that the
> > state is always reset after resuming.
> >
> > Signed-off-by: Fei Shao <fshao@chromium.org>
> > ---
>
> This is an interesting problem; were you able to root-cause why the silicon
> exhibits this behavior? Simply asserting reset should not cause it to draw
> additional power, let alone 14 mW. This almost sounds like a back-powering
> problem during suspend.
>
There was a fix for this behavior before so I didn't dig into it on
the silicon side.
I can ask internally and see if we can have Goodix to confirm this is
a known HW erratum.
> If this is truly expected behavior, is it sufficient to use the always_on
> constraint of the relevant regulator(s) to make this decision as opposed to
> introducing a new property?
>
That sounds good to me. IIUC, for the existing designs, the boards
that would set this property would also exclusively set
`regulator-always-on` in their supply, so that should suffice.
Let me revise the patch. Thanks!
Fei
> >
> > Documentation/devicetree/bindings/input/goodix,gt7375p.yaml | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> > index ce18d7dadae2..942acb286d77 100644
> > --- a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> > +++ b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> > @@ -43,6 +43,12 @@ properties:
> > itself as long as it allows the main board to make signals compatible
> > with what the touchscreen is expecting for its IO rails.
> >
> > + powered-in-suspend:
> > + description:
> > + This indicates that the touchscreen is powered in suspend, so the driver
> > + will not assert the reset GPIO in power-down to prevent power leakage.
> > + type: boolean
> > +
> > required:
> > - compatible
> > - reg
> > --
> > 2.40.0.634.g4ca3ef3211-goog
> >
>
> Kind regards,
> Jeff LaBundy
^ permalink raw reply
* W sprawie samochodu
From: Krystian Wieczorek @ 2023-04-19 8:45 UTC (permalink / raw)
To: linux-input
Dzień dobry,
chcielibyśmy zapewnić Państwu kompleksowe rozwiązania, jeśli chodzi o system monitoringu GPS.
Precyzyjne monitorowanie pojazdów na mapach cyfrowych, śledzenie ich parametrów eksploatacyjnych w czasie rzeczywistym oraz kontrola paliwa to kluczowe funkcjonalności naszego systemu.
Organizowanie pracy pracowników jest dzięki temu prostsze i bardziej efektywne, a oszczędności i optymalizacja w zakresie ponoszonych kosztów, mają dla każdego przedsiębiorcy ogromne znaczenie.
Dopasujemy naszą ofertę do Państwa oczekiwań i potrzeb organizacji. Czy moglibyśmy porozmawiać o naszej propozycji?
Pozdrawiam
Krystian Wieczorek
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox