Received-SPF: pass (google.com: domain of linux-kernel+bounces-21131-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <ebdc7d87-c0c4-9ee3-83a9-6c93e6fcea78@arm.com>
Date: Tue, 9 Jan 2024 16:48:57 +0000
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH 8/8] coresight: Remove unused stubs
Content-Language: en-US
To: Suzuki K Poulose <suzuki.poulose@arm.com>, coresight@lists.linaro.org
Cc: Mike Leach <mike.leach@linaro.org>,
 Alexander Shishkin <alexander.shishkin@linux.intel.com>,
 Maxime Coquelin <mcoquelin.stm32@gmail.com>,
 Alexandre Torgue <alexandre.torgue@foss.st.com>,
 linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
 linux-stm32@st-md-mailman.stormreply.com
References: <20231212155407.1429121-1-james.clark@arm.com>
 <20231212155407.1429121-9-james.clark@arm.com>
 <01c15dbe-57d3-4aa1-9d68-81a01ec2a3a7@arm.com>
From: James Clark <james.clark@arm.com>
In-Reply-To: <01c15dbe-57d3-4aa1-9d68-81a01ec2a3a7@arm.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


On 09/01/2024 10:38, Suzuki K Poulose wrote:
> Hi James
> 
> On 12/12/2023 15:54, James Clark wrote:
>> These are a bit annoying to keep up to date when the function signatures
>> change. But if CONFIG_CORESIGHT isn't enabled, then they're not used
>> anyway so just delete them.
>>
> 
> Have you tried building an arm32 kernel with this change in ? Looks like
> arch/arm/kernel/hw_breakpoint.c includes linux/coresight.h and a build
> with CONFIG_CORSIGHT=n might break the build ? So is

arm32 and CONFIG_CORESIGHT=n works because hw_breakpoint.c doesn't use
any of those symbols, only #defines that were outside the #if
IS_ENABLED(CONFIG_CORESIGHT), specifically CORESIGHT_UNLOCK.

> drivers/accel/habanalabs/common/habanalabs.h. Now, I am not sure if they

habanalabs is interesting, it depends on X86_64, but CONFIG_CORESIGHT
depends on ARM || ARM64, so I think we can assume it's also only looking
for #defines and inlines, and not actual code.

Either way I can't find any build config that actually ever built this,
meaning it's always been dead code. I would have expected some build
robot to have flagged an error by now as I've seen that on other
coresight patches.

> really need it (even if they do, we may be able to remove the dependency
> on the header file.
> 

They do really need it, also for the CORESIGHT_UNLOCK definition, but
not any functions.

> Suzuki
> 
>> Signed-off-by: James Clark <james.clark@arm.com>
>> ---
>>   include/linux/coresight.h | 79 ---------------------------------------
>>   1 file changed, 79 deletions(-)
>>
>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>> index 4400d554a16b..c5be46d7f85c 100644
>> --- a/include/linux/coresight.h
>> +++ b/include/linux/coresight.h
>> @@ -391,8 +391,6 @@ struct coresight_ops {
>>       const struct coresight_ops_helper *helper_ops;
>>   };
>>   -#if IS_ENABLED(CONFIG_CORESIGHT)
>> -
>>   static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa,
>>                             u32 offset)
>>   {
>> @@ -611,83 +609,6 @@ void coresight_relaxed_write64(struct
>> coresight_device *csdev,
>>                      u64 val, u32 offset);
>>   void coresight_write64(struct coresight_device *csdev, u64 val, u32
>> offset);
>>   -#else
>> -static inline struct coresight_device *
>> -coresight_register(struct coresight_desc *desc) { return NULL; }
>> -static inline void coresight_unregister(struct coresight_device
>> *csdev) {}
>> -static inline int
>> -coresight_enable_sysfs(struct coresight_device *csdev) { return
>> -ENOSYS; }
>> -static inline void coresight_disable_sysfs(struct coresight_device
>> *csdev) {}
>> -
>> -static inline int coresight_timeout(struct csdev_access *csa, u32
>> offset,
>> -                    int position, int value)
>> -{
>> -    return 1;
>> -}
>> -
>> -static inline int coresight_claim_device_unlocked(struct
>> coresight_device *csdev)
>> -{
>> -    return -EINVAL;
>> -}
>> -
>> -static inline int coresight_claim_device(struct coresight_device *csdev)
>> -{
>> -    return -EINVAL;
>> -}
>> -
>> -static inline void coresight_disclaim_device(struct coresight_device
>> *csdev) {}
>> -static inline void coresight_disclaim_device_unlocked(struct
>> coresight_device *csdev) {}
>> -
>> -static inline bool coresight_loses_context_with_cpu(struct device *dev)
>> -{
>> -    return false;
>> -}
>> -
>> -static inline u32 coresight_relaxed_read32(struct coresight_device
>> *csdev, u32 offset)
>> -{
>> -    WARN_ON_ONCE(1);
>> -    return 0;
>> -}
>> -
>> -static inline u32 coresight_read32(struct coresight_device *csdev,
>> u32 offset)
>> -{
>> -    WARN_ON_ONCE(1);
>> -    return 0;
>> -}
>> -
>> -static inline void coresight_write32(struct coresight_device *csdev,
>> u32 val, u32 offset)
>> -{
>> -}
>> -
>> -static inline void coresight_relaxed_write32(struct coresight_device
>> *csdev,
>> -                         u32 val, u32 offset)
>> -{
>> -}
>> -
>> -static inline u64 coresight_relaxed_read64(struct coresight_device
>> *csdev,
>> -                       u32 offset)
>> -{
>> -    WARN_ON_ONCE(1);
>> -    return 0;
>> -}
>> -
>> -static inline u64 coresight_read64(struct coresight_device *csdev,
>> u32 offset)
>> -{
>> -    WARN_ON_ONCE(1);
>> -    return 0;
>> -}
>> -
>> -static inline void coresight_relaxed_write64(struct coresight_device
>> *csdev,
>> -                         u64 val, u32 offset)
>> -{
>> -}
>> -
>> -static inline void coresight_write64(struct coresight_device *csdev,
>> u64 val, u32 offset)
>> -{
>> -}
>> -
>> -#endif        /* IS_ENABLED(CONFIG_CORESIGHT) */
>> -
>>   extern int coresight_get_cpu(struct device *dev);
>>     struct coresight_platform_data *coresight_get_platform_data(struct
>> device *dev);
>