[PATCH v2] hwmom/applesmc: add imacpro

[PATCH v2] hwmom/applesmc: add imacpro

[Atharva Tiwari]

Add iMacPro to whitelist as one iMacPro is released.
The iMacPro 1,1

Signed-off-by: Atharva Tiwari <evepolonium@gmail.com>
---
 drivers/hwmon/applesmc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index 7964b0e0c5e8..20e390d595e0 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -1373,6 +1373,10 @@ static const struct dmi_system_id applesmc_whitelist[] __initconst = {
 	  DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
 	  DMI_MATCH(DMI_PRODUCT_NAME, "iMac") },
 	},
+	{ applesmc_dmi_match, "Apple iMacPro", {
+	  DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
+	  DMI_MATCH(DMI_PRODUCT_NAME, "iMacPro") },
+	},
 	{ applesmc_dmi_match, "Apple Xserve", {
 	  DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
 	  DMI_MATCH(DMI_PRODUCT_NAME, "Xserve") },
-- 
2.39.5

Re: [PATCH v2] hwmom/applesmc: add imacpro

[Guenter Roeck]

On Thu, Jan 23, 2025 at 05:47:28PM +0530, Atharva Tiwari wrote:
> Add iMacPro to whitelist as one iMacPro is released.
> The iMacPro 1,1
> 
> Signed-off-by: Atharva Tiwari <evepolonium@gmail.com>

Applied to hwmon-next.

Note that the branch will only be pushed to linux-next after the commit
window closed.

Thanks,
Guenter

Re: [PATCH v2] hwmom/applesmc: add imacpro

[Guenter Roeck]

On 1/29/25 08:06, Guenter Roeck wrote:
> On Thu, Jan 23, 2025 at 05:47:28PM +0530, Atharva Tiwari wrote:
>> Add iMacPro to whitelist as one iMacPro is released.
>> The iMacPro 1,1
>>
>> Signed-off-by: Atharva Tiwari <evepolonium@gmail.com>
> 
> Applied to hwmon-next.
> 

I should add that I fixed the subject line (there is no hwmom subsystem)
and adjusted the description, so there is no need to comment on that.

Guenter

Re: [PATCH v2] hwmom/applesmc: add imacpro

[Aditya Garg]

Hi Guenter

Sorry to barge in like that but I have some issues with this here.

1. The iMacPro1,1 is a T2 Mac and the applesmc driver does not support T2 Macs upstream, thus making no point of adding this. We at t2linux have downstream patches to support T2 Macs, but we do not aim to upstream them since we wish to use a separate, and better driver for the SMC on T2 Macs.

2. The person who submitted the patch took it from the t2linux project, of which I am a part of, but didn't care to even credit the original author of the patch. I’m not sure how legal it is to redistribute the code in his own name and removing the original author’s name without taking permission from the author as per the GPL2 license, but I think the author should be credited. In fact the original patch was Signed-off-by the original author.

For reference, the original patch, which we never intended to be sent upstream, is given over here:

https://github.com/t2linux/linux-t2-patches/blob/main/3007-applesmc-Add-iMacPro-to-applesmc_whitelist.patch

I leave the fate of this to you now.

Regards
Aditya


> On 29 Jan 2025, at 9:36 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> 
> On Thu, Jan 23, 2025 at 05:47:28PM +0530, Atharva Tiwari wrote:
>> Add iMacPro to whitelist as one iMacPro is released.
>> The iMacPro 1,1
>> 
>> Signed-off-by: Atharva Tiwari <evepolonium@gmail.com>
> 
> Applied to hwmon-next.
> 
> Note that the branch will only be pushed to linux-next after the commit
> window closed.
> 
> Thanks,
> Guenter
> 

Re: [PATCH v2] hwmom/applesmc: add imacpro

[Guenter Roeck]

Hi Aditya,

On 1/29/25 10:22, Aditya Garg wrote:
> Hi Guenter
> 
> Sorry to barge in like that but I have some issues with this here.
> 

No worries. Thanks for stepping in.

> 1. The iMacPro1,1 is a T2 Mac and the applesmc driver does not support T2 Macs upstream, thus making no point of adding this. We at t2linux have downstream patches to support T2 Macs, but we do not aim to upstream them since we wish to use a separate, and better driver for the SMC on T2 Macs.
> 
> 2. The person who submitted the patch took it from the t2linux project, of which I am a part of, but didn't care to even credit the original author of the patch. I’m not sure how legal it is to redistribute the code in his own name and removing the original author’s name without taking permission from the author as per the GPL2 license, but I think the author should be credited. In fact the original patch was Signed-off-by the original author.
> 
> For reference, the original patch, which we never intended to be sent upstream, is given over here:
> 
> https://github.com/t2linux/linux-t2-patches/blob/main/3007-applesmc-Add-iMacPro-to-applesmc_whitelist.patch
> 

FWIW, that patch looks much better than this patch.

I am a bit confused, though: You are saying above that the patch is pointless.
Is that because context patches adding support for T2 are missing ?

Also, what about the patch at [1] ? It also adds t2 specific functionality,
and it seems to match another patch from the t2linux project.
Is that also missing some context ?

> I leave the fate of this to you now.
> 

Thanks for letting me know. I dropped the patch. I'll also wait for your feedback
about the patch at [1] before looking into it further.

Thanks,
Guenter

---
[1] https://patchwork.kernel.org/project/linux-hwmon/patch/20250120183343.3494-1-evepolonium@gmail.com/

Re: [PATCH v2] hwmom/applesmc: add imacpro

[Aditya Garg]

Hi

> On 30 Jan 2025, at 1:06 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> 
> Hi Aditya,
> 
>> On 1/29/25 10:22, Aditya Garg wrote:
>> Hi Guenter
>> Sorry to barge in like that but I have some issues with this here.
> 
> No worries. Thanks for stepping in.
> 
>> 1. The iMacPro1,1 is a T2 Mac and the applesmc driver does not support T2 Macs upstream, thus making no point of adding this. We at t2linux have downstream patches to support T2 Macs, but we do not aim to upstream them since we wish to use a separate, and better driver for the SMC on T2 Macs.
>> 2. The person who submitted the patch took it from the t2linux project, of which I am a part of, but didn't care to even credit the original author of the patch. I’m not sure how legal it is to redistribute the code in his own name and removing the original author’s name without taking permission from the author as per the GPL2 license, but I think the author should be credited. In fact the original patch was Signed-off-by the original author.
>> For reference, the original patch, which we never intended to be sent upstream, is given over here:
>> https://github.com/t2linux/linux-t2-patches/blob/main/3007-applesmc-Add-iMacPro-to-applesmc_whitelist.patch
> 
> FWIW, that patch looks much better than this patch.
> 
> I am a bit confused, though: You are saying above that the patch is pointless.
> Is that because context patches adding support for T2 are missing ?
> 
> Also, what about the patch at [1] ? It also adds t2 specific functionality,
> and it seems to match another patch from the t2linux project.
> Is that also missing some context ?
> 

As far as patch in [1] in considered:

1. The patch alone cannot make fan work on T2 Macs. As you saw the other T2 patches, the 300x patch series in total is needed for T2 support, the reason is mentioned in the patch description of individual patch, which you may read if interested.

2. The patch becomes pointless again, since the driver still doesn't support T2 Macs. Ideally the whole patch series should be submitted.

3. We do not plan to send the whole patch series to add T2 support in applesmc, since Asahi Linux has a better SMC driver and we aim to use that. Just in case we change our decision to use applesmc, the whole series with proper credits shall be sent upstream, and not random patches without crediting the original people who wrote the code.

4. This patch was originally authored by Paul Pawlowski, and not the submitter, which you might have noticed when you saw the t2linux patch repo (it's the same repo which I sent in the previous email). The author had not yet signed-off the patch, thus requiring permission from the author as well.

I've also noticed the Aun-Ali Zaidi, the who the submitter has added in the signed-off-by was not Cced, which is kinda strange.

Again, like previous patch, the final call stays with you being a maintainer.

Thanks
Aditya

>> I leave the fate of this to you now.
> 
> Thanks for letting me know. I dropped the patch. I'll also wait for your feedback
> about the patch at [1] before looking into it further.
> 
> Thanks,
> Guenter
> 
> ---
> [1] https://patchwork.kernel.org/project/linux-hwmon/patch/20250120183343.3494-1-evepolonium@gmail.com/
> 

Re: [PATCH v2] hwmom/applesmc: add imacpro

[Guenter Roeck]

On 1/29/25 19:54, Aditya Garg wrote:
> Hi
> 
>> On 30 Jan 2025, at 1:06 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> Hi Aditya,
>>
>>> On 1/29/25 10:22, Aditya Garg wrote:
>>> Hi Guenter
>>> Sorry to barge in like that but I have some issues with this here.
>>
>> No worries. Thanks for stepping in.
>>
>>> 1. The iMacPro1,1 is a T2 Mac and the applesmc driver does not support T2 Macs upstream, thus making no point of adding this. We at t2linux have downstream patches to support T2 Macs, but we do not aim to upstream them since we wish to use a separate, and better driver for the SMC on T2 Macs.
>>> 2. The person who submitted the patch took it from the t2linux project, of which I am a part of, but didn't care to even credit the original author of the patch. I’m not sure how legal it is to redistribute the code in his own name and removing the original author’s name without taking permission from the author as per the GPL2 license, but I think the author should be credited. In fact the original patch was Signed-off-by the original author.
>>> For reference, the original patch, which we never intended to be sent upstream, is given over here:
>>> https://github.com/t2linux/linux-t2-patches/blob/main/3007-applesmc-Add-iMacPro-to-applesmc_whitelist.patch
>>
>> FWIW, that patch looks much better than this patch.
>>
>> I am a bit confused, though: You are saying above that the patch is pointless.
>> Is that because context patches adding support for T2 are missing ?
>>
>> Also, what about the patch at [1] ? It also adds t2 specific functionality,
>> and it seems to match another patch from the t2linux project.
>> Is that also missing some context ?
>>
> 
> As far as patch in [1] in considered:
> 
> 1. The patch alone cannot make fan work on T2 Macs. As you saw the other T2 patches, the 300x patch series in total is needed for T2 support, the reason is mentioned in the patch description of individual patch, which you may read if interested.
> 
> 2. The patch becomes pointless again, since the driver still doesn't support T2 Macs. Ideally the whole patch series should be submitted.
> 
> 3. We do not plan to send the whole patch series to add T2 support in applesmc, since Asahi Linux has a better SMC driver and we aim to use that. Just in case we change our decision to use applesmc, the whole series with proper credits shall be sent upstream, and not random patches without crediting the original people who wrote the code.
> 
> 4. This patch was originally authored by Paul Pawlowski, and not the submitter, which you might have noticed when you saw the t2linux patch repo (it's the same repo which I sent in the previous email). The author had not yet signed-off the patch, thus requiring permission from the author as well.
> 
> I've also noticed the Aun-Ali Zaidi, the who the submitter has added in the signed-off-by was not Cced, which is kinda strange.
> 
> Again, like previous patch, the final call stays with you being a maintainer.
> 

That is a simple call: I'll reject that patch. Thanks for letting me know.

Guenter

Re: [PATCH v2] hwmom/applesmc: add imacpro

[Aditya Garg]

Thanks a lot Guenter for the timely action!

> On 30 Jan 2025, at 10:37 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> 
> On 1/29/25 19:54, Aditya Garg wrote:
>> Hi
>>>> On 30 Jan 2025, at 1:06 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>>> 
>>> Hi Aditya,
>>> 
>>>> On 1/29/25 10:22, Aditya Garg wrote:
>>>> Hi Guenter
>>>> Sorry to barge in like that but I have some issues with this here.
>>> 
>>> No worries. Thanks for stepping in.
>>> 
>>>> 1. The iMacPro1,1 is a T2 Mac and the applesmc driver does not support T2 Macs upstream, thus making no point of adding this. We at t2linux have downstream patches to support T2 Macs, but we do not aim to upstream them since we wish to use a separate, and better driver for the SMC on T2 Macs.
>>>> 2. The person who submitted the patch took it from the t2linux project, of which I am a part of, but didn't care to even credit the original author of the patch. I’m not sure how legal it is to redistribute the code in his own name and removing the original author’s name without taking permission from the author as per the GPL2 license, but I think the author should be credited. In fact the original patch was Signed-off-by the original author.
>>>> For reference, the original patch, which we never intended to be sent upstream, is given over here:
>>>> https://github.com/t2linux/linux-t2-patches/blob/main/3007-applesmc-Add-iMacPro-to-applesmc_whitelist.patch
>>> 
>>> FWIW, that patch looks much better than this patch.
>>> 
>>> I am a bit confused, though: You are saying above that the patch is pointless.
>>> Is that because context patches adding support for T2 are missing ?
>>> 
>>> Also, what about the patch at [1] ? It also adds t2 specific functionality,
>>> and it seems to match another patch from the t2linux project.
>>> Is that also missing some context ?
>>> 
>> As far as patch in [1] in considered:
>> 1. The patch alone cannot make fan work on T2 Macs. As you saw the other T2 patches, the 300x patch series in total is needed for T2 support, the reason is mentioned in the patch description of individual patch, which you may read if interested.
>> 2. The patch becomes pointless again, since the driver still doesn't support T2 Macs. Ideally the whole patch series should be submitted.
>> 3. We do not plan to send the whole patch series to add T2 support in applesmc, since Asahi Linux has a better SMC driver and we aim to use that. Just in case we change our decision to use applesmc, the whole series with proper credits shall be sent upstream, and not random patches without crediting the original people who wrote the code.
>> 4. This patch was originally authored by Paul Pawlowski, and not the submitter, which you might have noticed when you saw the t2linux patch repo (it's the same repo which I sent in the previous email). The author had not yet signed-off the patch, thus requiring permission from the author as well.
>> I've also noticed the Aun-Ali Zaidi, the who the submitter has added in the signed-off-by was not Cced, which is kinda strange.
>> Again, like previous patch, the final call stays with you being a maintainer.
> 
> That is a simple call: I'll reject that patch. Thanks for letting me know.
> 
> Guenter
>