From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753883AbdL1OA2 (ORCPT ); Thu, 28 Dec 2017 09:00:28 -0500 Received: from mail3-relais-sop.national.inria.fr ([192.134.164.104]:20471 "EHLO mail3-relais-sop.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753792AbdL1OAZ (ORCPT ); Thu, 28 Dec 2017 09:00:25 -0500 X-IronPort-AV: E=Sophos;i="5.45,471,1508796000"; d="scan'208";a="249624340" Date: Thu, 28 Dec 2017 15:00:22 +0100 (CET) From: Julia Lawall X-X-Sender: jll@hadrien To: Ingo Molnar cc: Julia Lawall , Andy Shevchenko , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, Andi Kleen , linux-kernel@vger.kernel.org, Darren Hart , platform-driver-x86@vger.kernel.org, Bhumika Goyal , Linus Torvalds , Peter Zijlstra , Andrew Morton Subject: Re: [PATCH v1] x86/platform/intel-mid: Revert "Make 'bt_sfi_data' const" In-Reply-To: <20171228125341.46xng2mzagksfyhr@gmail.com> Message-ID: References: <20171228100801.67744-1-andriy.shevchenko@linux.intel.com> <20171228102812.cm7yylrrq2omfw5y@gmail.com> <1514463164.7000.328.camel@linux.intel.com> <20171228121711.xwydozmtj5jhwkgj@gmail.com> <20171228122931.ie5dggc43g6zjkkc@gmail.com> <20171228125341.46xng2mzagksfyhr@gmail.com> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 28 Dec 2017, Ingo Molnar wrote: > > * Julia Lawall wrote: > > > > > [...] There does seem to be a few cases where the field actually does hold an > > > > integer. I guess this is not a problem? > > > > > > Could you point to such an example? > > > > drivers/thermal/intel_soc_dts_thermal.c:#define BYT_SOC_DTS_APIC_IRQ 86 > > > > and then: > > > > static const struct x86_cpu_id soc_thermal_ids[] = { > > { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT1, 0, > > BYT_SOC_DTS_APIC_IRQ}, > > {} > > }; > > > > and finally: > > > > soc_dts_thres_irq = (int)match_cpu->driver_data; > > > > Also: > > > > arch/x86/kernel/apic/apic.c > > > > #define DEADLINE_MODEL_MATCH_REV(model, rev) \ > > { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned long)rev > > } > > > > DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_BROADWELL_X, 0x0b000020), > > DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_HASWELL_CORE, 0x22), > > etc. (all 2-digit numbers in the remaining case). > > Ok - I think in these cases the resulting long->pointer type conversion is a _lot_ > less dangerous than the pointer->long conversion which caused the regression. > > So unless the resulting code is excessively ugly, this feels like the right > approach to me. The problem is that this case will inevitably have a cast somewhere. Many of the values put into the driver_data field really are const, so the type has to be const void *. When the value is extracted from the structure, there will thus need to be a cast on it, and the current cast ddata = (struct bt_sfi_data *)id->driver_data; works fine, whether the original structure is const or not. I also got a couple of complaints about non-pointer types: arch/x86/kernel/apic/apic.c:621:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] rev = (u32)m->driver_data; drivers/thermal/intel_soc_dts_thermal.c:68:22: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] soc_dts_thres_irq = (int)match_cpu->driver_data; julia