From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.8]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5777B55C28 for ; Thu, 22 Feb 2024 12:23:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.8 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708604632; cv=none; b=ss1f4xWrFabxKqfOOQS92QdWp4lNVGLCgMekbECWNVdh2AL9SQwSPgfIiGRS9SDxZJjxQODBq8KRTYxmYQqtLkB9bnbUiDIOVltkhyBkctpX+fUuTg0nzxX6Nw5pgcFdcrhsTvde4NAU2uklvhC32biHoaKXpYNUZJcoRu7KyTs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708604632; c=relaxed/simple; bh=7qo3+hrMoVGrilqWdP7PcIKv3mA4uvCCCzTog8JqbsI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=PwcZU20w4JdTbUiwO/WtMN+AHm77pLExg+sMMTau500mA4H+Wmp7SZD6yqNt1+UCdpB9E7pjlymfjSUzUcHWPchFgPwsO71QZvlWwORoR0EK2Sr6SyeEZ/xKYCmHpibhu7DRkSg0+UJZpCPX5jU3kxY9y2IPKiazXlpwMHyJCDo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=none smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=CdtoxS5Z; arc=none smtp.client-ip=192.198.163.8 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="CdtoxS5Z" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1708604630; x=1740140630; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=7qo3+hrMoVGrilqWdP7PcIKv3mA4uvCCCzTog8JqbsI=; b=CdtoxS5Zh4PrzX41Hl/VENgSRUIsVwRlOsNmtmM3/vYrCCmlpmUf1BvQ +er9RGEq812jKyi1bkSR281NDabSNWyC6LHGtDTgWUZWK8B8PB8hjgzFB IlLYXKX1dgDdOvOgkdqSrwD7Lsz18p6dXGKxXxvU67B/WsCuo1itH2pRj oPjibHA1ugpqtL2j9X9odcCytkYVfq3DCoevktqGEwX7CfLCcxw0u0jLG 7rjawBWtbsRvfJh+ewzXF/SGU9ADJQFuvaMv9GtIA7lWfgbMNZw2w50Fq sKCu/CXX/eSdvkDZ4/MDSERyAWVQhLf/a4O3fJFtKAO2AX0PTdUOMK5fu g==; X-IronPort-AV: E=McAfee;i="6600,9927,10991"; a="20376894" X-IronPort-AV: E=Sophos;i="6.06,177,1705392000"; d="scan'208";a="20376894" Received: from orviesa006.jf.intel.com ([10.64.159.146]) by fmvoesa102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Feb 2024 04:23:49 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.06,177,1705392000"; d="scan'208";a="5848090" Received: from turnipsi.fi.intel.com (HELO kekkonen.fi.intel.com) ([10.237.72.44]) by orviesa006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Feb 2024 04:23:48 -0800 Received: from kekkonen.localdomain (localhost [127.0.0.1]) by kekkonen.fi.intel.com (Postfix) with ESMTP id 9492C11F81D; Thu, 22 Feb 2024 14:23:44 +0200 (EET) Date: Thu, 22 Feb 2024 12:23:44 +0000 From: Sakari Ailus To: Andy Shevchenko Cc: linux-acpi@vger.kernel.org, "Rafael J. Wysocki" , Len Brown Subject: Re: [PATCH v2 2/2] ACPI: property: Ignore bad graph port nodes on Dell XPS 9315 Message-ID: References: <20240213134606.383817-1-sakari.ailus@linux.intel.com> <20240213134606.383817-3-sakari.ailus@linux.intel.com> Precedence: bulk X-Mailing-List: linux-acpi@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Wed, Feb 21, 2024 at 05:33:36PM +0200, Andy Shevchenko wrote: > On Wed, Feb 21, 2024 at 8:56 AM Sakari Ailus > wrote: > > I think Rafael has already applied this but I can send a new patch... > > Depends if it's final or can be dropped. > If the former is the case, follow ups, please. > > > On Wed, Feb 21, 2024 at 12:21:13AM +0200, andy.shevchenko@gmail.com wrote: > > > Tue, Feb 13, 2024 at 03:46:06PM +0200, Sakari Ailus kirjoitti: > > ... > > > > > void acpi_mipi_scan_crs_csi2(void); > > > > void acpi_mipi_init_crs_csi2_swnodes(void); > > > > void acpi_mipi_crs_csi2_cleanup(void); > > > > > > + blank line? > > > > The usa of blank lines elsewhere in the file is consistent with the above. > > These are all related. > > Naming does not suggest that. I see either naming or semantic tights > issues here. Hence the proposal to have a blank line. > > > > > +bool acpi_graph_ignore_port(acpi_handle handle); > > ... > > > > > +static const struct dmi_system_id dmi_ignore_port_nodes[] = { > > > > + { > > > > + .matches = { > > > > + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dell Inc."), > > > > + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "XPS 9315"), > > > > + }, > > > > + }, > > > > + { 0 } > > > > > > 0 is not needed. Moreover, it's a bit unusual. > > > > But also required by the C standard. > > We have a lot of code that doesn't use that (yet this is valid C23, > and yes I know that we rely on C11). The current compilers support it and I guess we'll switch to C23 at some point. I can drop the 0. > > > > > +}; > > ... > > > > > +static const char *strnext(const char *s1, const char *s2) > > > > +{ > > > > + s1 = strstr(s1, s2); > > > > + > > > > + if (!s1) > > > > + return NULL; > > > > + > > > > + return s1 + strlen(s2); > > > > +} > > > > > > NIH str_has_prefix() ? > > > > It doesn't do the same thing. Yes, it could be used, but the code looks > > cleaner with this. > > "Not Invented Here" was even at Nokia times, why do we repeat our mistakes? As I noted previously, while str_has_prefix() could be used, it would make the code in the caller harder to read. Changes may be needed to that code later on as this isn't the only system with the issue so readability does count. > > ... > > > > > +/** > > > > + * acpi_graph_ignore_port - Tell whether a port node should be ignored > > > > + * @handle: The ACPI handle of the node (which may be a port node) > > > > + * > > > > + * Returns true if a port node should be ignored and the data to that should > > > > > > Please, run kernel-doc validation and fix the warnings. > > > > Running kernel-doc on the file doesn't produce any here. > > You haven't run it correctly? > > $ scripts/kernel-doc -v -none -Wall drivers/acpi/mipi-disco-img.c > drivers/acpi/mipi-disco-img.c:163: info: Scanning doc for function > acpi_mipi_check_crs_csi2 > drivers/acpi/mipi-disco-img.c:384: info: Scanning doc for function > acpi_mipi_scan_crs_csi2 > drivers/acpi/mipi-disco-img.c:703: info: Scanning doc for function > acpi_mipi_init_crs_csi2_swnodes > drivers/acpi/mipi-disco-img.c:718: info: Scanning doc for function > acpi_mipi_crs_csi2_cleanup > drivers/acpi/mipi-disco-img.c:749: info: Scanning doc for function > acpi_graph_ignore_port > drivers/acpi/mipi-disco-img.c:759: warning: No description found for > return value of 'acpi_graph_ignore_port' > 1 warnings Ah, I guess it was -Wall option that was required to produce the warning. I'll address it. > > > > > + * come from other sources instead (Windows ACPI definitions and > > > > + * ipu-bridge). This is currently used to ignore bad port nodes related to IPU6 > > > > + * ("IPU?") and camera sensor devices ("LNK?") in certain Dell systems with > > > > + * Intel VSC. > > > > + */ > -- Regards, Sakari Ailus