mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Arnd Bergmann" <arnd@arndb.de>
To: david@ixit.cz, "Catalin Marinas" <catalin.marinas@arm.com>,
	"Will Deacon" <will@kernel.org>,
	"Casey Connolly" <casey.connolly@linaro.org>,
	"Casey Connolly" <casey@connolly.tech>,
	"Joel Selvaraj" <foss@joelselvaraj.com>,
	"Alexander Martinz" <amartinz@shiftphones.com>,
	"Dzmitry Sankouski" <dsankouski@gmail.com>,
	"Pablo Correa Gómez" <ablocorrea@hotmail.com>
Cc: "Guido Günther" <agx@sigxcpu.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, phone-devel@vger.kernel.org
Subject: Re: [PATCH QUESTION] arm64: configs: Add Snapdragon 845 config fragment
Date: Wed, 03 Dec 2025 22:35:36 +0100	[thread overview]
Message-ID: <8f8baac8-3b1a-442d-a64f-5a90f9e30552@app.fastmail.com> (raw)
In-Reply-To: <20251126-sdm845-config-question-v1-1-ffa91ed53095@ixit.cz>

On Wed, Nov 26, 2025, at 17:19, David Heidelberg via B4 Relay wrote:
>
> This patch is a question, if it would be viable to introduce this
> configuration fragment as part of mainline kernel, so keeping it in sync
> with recent kernel updates would be more straighforward.

I don't mind the idea of having additional fragments for specific
purposes, but as others have said this one does not seem that useful.

I think it does too many things at once and is still too specific to
a relatively uncommon usecase. Having an SDM845 specific fragment is
clearly too narrow, but I can see us adding a fragment for phones
overall, which would then turn on options for Qualcomm, Mediatek
and Samsung based phones.

> +# Samsung S9 SM-G9600(starqltechn)
> +CONFIG_SND_SOC_MAX98512=y
> +CONFIG_SND_SOC_MAXIM_DSM=y
> +CONFIG_SND_SOC_MAXIM_DSM_CAL=y
> +CONFIG_DRM_PANEL_SAMSUNG_S6E3HA8=y
> +CONFIG_TOUCHSCREEN_S6SY761=m
> +CONFIG_MFD_SEC_CORE=m
> +CONFIG_REGULATOR_S2DOS05=m
> +CONFIG_MFD_MAX77705=m
> +CONFIG_LEDS_MAX77705=m
> +CONFIG_CHARGER_MAX77705=m
> +CONFIG_BATTERY_MAX17042=m
> +CONFIG_INPUT_MAX77693_HAPTIC=m
> +CONFIG_PWM_CLK=m
> +
> +# SHIFT6mq
> +CONFIG_DRM_PANEL_VISIONOX_RM69299=y
> +CONFIG_SND_SOC_TFA989X=m

I think overall we can be fairly liberal with adding
more loadable modules to the common defconfig, but much
less so the built-in drivers.

> +# SOC
> +CONFIG_FORCE_NR_CPUS=y
> +CONFIG_NR_CPUS=8

This would break other devices that have more CPUs, which in turn
makes it impossible to combine the fragment with other fragments.

> +CONFIG_SCSI_UFS_QCOM=y
> +CONFIG_QCOM_GSBI=y
> +CONFIG_QCOM_LLCC=y
> +CONFIG_QCOM_OCMEM=y
> +CONFIG_QCOM_RMTFS_MEM=y
> +CONFIG_QCOM_SOCINFO=y
> +CONFIG_QCOM_WCNSS_CTRL=y
> +CONFIG_QCOM_APR=y
> +CONFIG_POWER_RESET_QCOM_PON=y
> +CONFIG_QCOM_SPMI_TEMP_ALARM=y
> +CONFIG_QCOM_LMH=y
> +CONFIG_SCHED_CLUSTER=y
> +CONFIG_SND_SOC_QDSP6_Q6VOICE=m
> +CONFIG_SCSI_UFS_BSG=y
> +CONFIG_PHY_QCOM_QMP_PCIE=y
> +CONFIG_BACKLIGHT_CLASS_DEVICE=y
> +CONFIG_INTERCONNECT_QCOM_OSM_L3=y

Again, these should mosely be loadable modules.

> +# Notification LED
> +# Must be builtin as it won't be automatically modprobed
> +CONFIG_LEDS_TRIGGER_PATTERN=y

This sounds like bug that we should fix and not work around.

> +# Graphics
> +CONFIG_DRM=y
> +CONFIG_FB_SIMPLE=y

I don't want to enable more fbdev drivers. Right now the
only one we enable is FB_EFI, and we should probably replace
that with DRM_SIMPLEDRM in order to turn off CONFIG_FB
entirely.

> +# Power management
> +CONFIG_PM_AUTOSLEEP=y
> +CONFIG_PM_WAKELOCKS=y
> +
> +# MGLRU
> +CONFIG_LRU_GEN=y
> +CONFIG_LRU_GEN_ENABLED=y
> +
> +# Usage clamping (scale CPU for specific tasks)
> +CONFIG_UCLAMP_TASK=y
> +CONFIG_UCLAMP_TASK_GROUP=y

We can discuss changing the defaults for these in the defconfig,
but this doesn't seem to belong with the other stuff.

> +# HID/Input
> +CONFIG_LCD_CLASS_DEVICE=m

This is not an input option

> +CONFIG_I2C_HID=y
> +CONFIG_HID_GENERIC=m
> +CONFIG_UHID=m
> +CONFIG_USB_HID=m
> +CONFIG_INPUT_EVDEV=y
> +CONFIG_BT_HIDP=m
> +CONFIG_INPUT_JOYDEV=m
> +CONFIG_HID_ACCUTOUCH=m
> +CONFIG_HID_ACRUX=m
> +CONFIG_HID_ACRUX_FF=y
> +CONFIG_HID_ALPS=m
> +CONFIG_HID_APPLEIR=m

This looks like you are just enabling every single USB HID 
driver, which is probably not something we want in a commonly
used fragment, though having a well-maintained fragment for
popular pluggable USB devices may be useful for others as well.

> +# Qcom stuff
> +CONFIG_RPMSG_CHAR=y
> +CONFIG_QCOM_Q6V5_ADSP=m
> +CONFIG_BT_RFCOMM=m
> +CONFIG_BT_RFCOMM_TTY=y
> +CONFIG_BT_BNEP=m
> +CONFIG_BT_BNEP_MC_FILTER=y
> +CONFIG_BT_BNEP_PROTO_FILTER=y
> +CONFIG_BT_HS=y
> +CONFIG_BT_LE=y
> +CONFIG_HID_BATTERY_STRENGTH=y
> +CONFIG_HIDRAW=y
> +CONFIG_QCOM_COINCELL=m
> +CONFIG_QCOM_FASTRPC=m
> +CONFIG_QCOM_SPMI_VADC=y
> +CONFIG_QCOM_SPMI_ADC5=y
> +CONFIG_PHY_QCOM_QMP=y
> +CONFIG_PHY_QCOM_QUSB2=y
> +CONFIG_PHY_QCOM_QMP_UFS=y
> +CONFIG_TYPEC=y
> +CONFIG_PHY_QCOM_QMP_COMBO=y
> +CONFIG_LEDS_CLASS_FLASH=y
> +CONFIG_TCP_CONG_ADVANCED=y
> +CONFIG_TCP_CONG_WESTWOOD=y
> +CONFIG_DEFAULT_WESTWOOD=y
> +CONFIG_BLK_DEV_RAM=y
> +CONFIG_BLK_DEV_RAM_SIZE=8192
> +CONFIG_BTRFS_FS=y
> +CONFIG_BTRFS_FS_POSIX_ACL=y
> +CONFIG_F2FS_FS=y

Most of these don't look like "Qcom stuff" to me. In particular
BTRFS is probably not something you'd even want to enable on
a phone, though that could be part of a generic distro config
that we have discussed several times in the past.

> +# WLAN debugging
> +CONFIG_ATH10K_DEBUG=y
> +CONFIG_ATH10K_DEBUGFS=y
> +CONFIG_ATH10K_SPECTRAL=y

This particular one seem arbitrary, but having a "debugging"
fragment is something others may find helpful as well. The
hard part there is figuring out which debug options are the
most helpful for the least overhead, as everyone is debugging
different bugs.

> +# Disable all unrelated stuffs afaik
> +CONFIG_ARCH_BLAIZE=n
> +CONFIG_ARCH_SPARX5=n
> +CONFIG_HIBERNATION=n
> +CONFIG_FW_LOADER_USER_HELPER=n
> +CONFIG_FW_LOADER_USER_HELPER_FALLBACK=n
> +CONFIG_BLK_DEV_NVME=n

It feels wrong to turn things off in the same fragment
that turns other things on.

      Arnd

  parent reply	other threads:[~2025-12-03 21:36 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-26 16:19 David Heidelberg via B4 Relay
2025-11-26 18:10 ` Catalin Marinas
2025-11-26 23:10   ` David Heidelberg
2025-11-26 19:03 ` Casey Connolly
2025-11-26 22:33   ` David Heidelberg
2025-11-27 19:52   ` Guido Günther
2025-12-03 21:35 ` Arnd Bergmann [this message]
2025-12-11 10:41   ` David Heidelberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8f8baac8-3b1a-442d-a64f-5a90f9e30552@app.fastmail.com \
    --to=arnd@arndb.de \
    --cc=ablocorrea@hotmail.com \
    --cc=agx@sigxcpu.org \
    --cc=amartinz@shiftphones.com \
    --cc=casey.connolly@linaro.org \
    --cc=casey@connolly.tech \
    --cc=catalin.marinas@arm.com \
    --cc=david@ixit.cz \
    --cc=dsankouski@gmail.com \
    --cc=foss@joelselvaraj.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=phone-devel@vger.kernel.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®