Message-ID: <07f6a7ce-ff22-695b-f843-d0d8ba275ae7@intel.com>
Date:   Mon, 6 Mar 2023 11:30:34 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH net-next 2/6] net: ipa: kill gsi->virt_raw
Content-Language: en-US
To:     Alex Elder <elder@ieee.org>
CC:     Alex Elder <elder@linaro.org>, <davem@davemloft.net>,
        <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
        <caleb.connolly@linaro.org>, <mka@chromium.org>,
        <evgreen@chromium.org>, <andersson@kernel.org>,
        <quic_cpratapa@quicinc.com>, <quic_avuyyuru@quicinc.com>,
        <quic_jponduru@quicinc.com>, <quic_subashab@quicinc.com>,
        <elder@kernel.org>, <netdev@vger.kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20230215195352.755744-1-elder@linaro.org>
 <20230215195352.755744-3-elder@linaro.org>
 <b0b2ae77-3311-34c8-d1a2-c6f30eca3f1e@intel.com>
 <c76bbb06-b6b0-8dae-965f-95e8af3634b6@linaro.org>
 <4c92160f-b2ea-c5ef-5647-6078ab47e518@intel.com>
 <a919afca-d33e-618d-5db3-17a08d90e8af@linaro.org>
 <5d90b252-c650-9908-05d3-fbbfdf47aa38@ieee.org>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <5d90b252-c650-9908-05d3-fbbfdf47aa38@ieee.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TTR5RWs2ZlY2djZyZzM5dEFTUGhqTkNWQmc5RWtqVVJJd1Q3c283UXBzcUd4?=
 =?utf-8?B?ZFp6a1Jsc3U0MTk1TnhqbGdiRXBWVDVCeWNPTUJycGNoaEpqRk00aTY0aTgw?=
 =?utf-8?B?NXJxbHB1QncyYWtnSkRicll3aGhKOEE4VEdSUEsrSUpjT2o4QXVxUENOZUNG?=
 =?utf-8?B?aGtvZjJqcEw2dGhNUktSQS9ZaEI0cnRhcVNvamsyVVp6dlY0TmhIbUZnditY?=
 =?utf-8?B?Tnp5ZkhGUjNDaksxakp2S2hlQ2V2bkdFcXFlbDA5eUZQL2cvVUdJSEtFYnRJ?=
 =?utf-8?B?bkdlcUtOYXRVQTlidCs1cVNjZndvcEpsbFhaaSttYVlvdzJYcTVWckVUN0Fp?=
 =?utf-8?B?cU05RTl3anJET1R5SUZQVUhTcEZTdytjNjNqd095Wnk5UXA3NkdXRE9pQXZM?=
 =?utf-8?B?V3Vmd3BWQ3BGZ2tDZGZxQWZMR3ByZ2JBb0E3VHpIc2xTdmRpMWVScmp4KzVF?=
 =?utf-8?B?VC9uN1ZPSHRFeFlpeCtCOGJYeHc0QURHNElydjlET2U0dVFCcmNIcHltRkw0?=
 =?utf-8?B?czhENXpiUzdnbnFRQnozMnVnNHdFUGZ1S1AzY1prSjJkQXpYVncyRGNPd0I3?=
 =?utf-8?B?R1dEbWtFRzdqRnNnSzBINDFiWHpsRE9PK0J3MXorN1B1NnZKWVZwUjFnQjZj?=
 =?utf-8?B?UlJHWHoxNExZempWY1pId09JeGF0a1E5Y0NETGFkZ1BmdDBvVDJBdHRCMDBJ?=
 =?utf-8?B?T25UeTlWQjdpQzE5ei9QZ1VQak8rTitqNFVvc1V0WC9DRVhmZEhmc2RXQmxK?=
 =?utf-8?B?Yis1c1JzUHBmakczSlpaL0lzVXMxUGpJNUVGMVdjZVAyMHN0MW0yRGM0QnVu?=
 =?utf-8?B?NzlQQkgrSGV4eklMUUlONmxya0wyQWNwc2hDRE8wQUJsV1pqSU9NM2wrWVdE?=
 =?utf-8?B?VTdnTUNWWG5tcTBhWEVJUEM4N0NMbXg4WDFreWsxVmxuMXVqbWljcEN1Zmk5?=
 =?utf-8?B?Ty9GRnJoeDdCUFdlRlRDcDhxek04OGU5d3AzQXgxRE90Y0l2Rmk4VlJkSVZZ?=
 =?utf-8?B?TVhJelNDOTI3eCszaVQxQ0xLMk93ZFBMVElOQjNtbk5EUVM5dU1FdjBZRXpC?=
 =?utf-8?B?SlFnSE1QQkdhR1NSWVY3YTd1VTR6b0VJSE5scDIrcTZkYUF0M2dUcUFDMzY4?=
 =?utf-8?B?eGlvVXlQWEt1czlEN3BPSmhxdTk3bGM0dzZvTHpKVzI4NXBDS1dKL053ZkJo?=
 =?utf-8?B?T1BjWTVuRGJESndPbzJoOEE2R0xkVk5JOUo2NURuNzQyRmQ2UWwvQ0dmV043?=
 =?utf-8?B?T0VQZG5IS2ROVUNxQ3pKbjloSEduMTJNMHRlR1B2VGVINHptKzR6QjRhazI2?=
 =?utf-8?B?Q0NSUEpUb3FRam8rLzY4NzJ5azVTR0pPbk82MFFML2hZRHBmQ1BFQTdaakUy?=
 =?utf-8?B?blJrL25NUVNDRnpGVGhqeUwxT0U0OVRWRFEzdDc1Nmp4OHNSTmQ2SkRJMWps?=
 =?utf-8?B?OVJqZVlIblNtS2NOc045QmwrNkJtazBFZ0kwOE5KdXg4NXZVS0IvWUNnZ1pC?=
 =?utf-8?B?ckErUktXZjJ1Q1l6MzVPejZXblRHTFJTL3ZUQVdRaHJEVnJudm5MMElMMWcr?=
 =?utf-8?B?blFQVGIwelhhT2RnRm9iZStKQnRoMjlKV1pwLzM3djBvWTBSMjBORi9MNnBK?=
 =?utf-8?B?Qzl2Z1pVTm55NEFNcjRQZnNNYmx5emN4YWhMVXgyeFhaMTJIcTdxVGhYRnZi?=
 =?utf-8?B?cmJIdXVoQ1RGb3lzSUV2cEZiKzVVUStndHRXZW00eGVXSzZ2Ry96bExKM1lD?=
 =?utf-8?B?V21DUFBVa3VKcGQ5OFk0WlNiSFIzbHovRlR1YXhXcmVET3ZIK0Z0UWNHYTM2?=
 =?utf-8?B?enVVMFBjL1Myd1JpakJVMzRUOXJPNkpBN2UyeERacGRrTlFGcjY0eUJXSk9z?=
 =?utf-8?B?bU1Vam1VVHdtTUFqWHFUYzFaZlhoOXZIU3VBdDk3Vk9tVWF5RTc5aFFQaHhG?=
 =?utf-8?B?K1gra2lGQnhpY0dHZ3pqRG02WXBlOVhzQVI3RXpFOXRhNC84eXNlU1FMdzJ0?=
 =?utf-8?B?SEFSVy9JOGppUmVOdEc3OWxqS0REWDJVSytPanZkTXp2KzBMYTU1RlNDanht?=
 =?utf-8?B?eFBSZnFtRHdVcnJwR1IxM202MmVDRWxXaDdwMVQ4WDNqU2psa2lSTWZoank4?=
 =?utf-8?B?b1hUdVYwZW5qN2ZpT3d3M1FtK0p5NzdhVXM4RmxLT0ZuM1BFODF0eTdMcEZL?=
 =?utf-8?Q?35w2nWaYsmPV2HCEC8f4AF8=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 4bd01f23-5b15-4cc7-c7ec-08db1e2e023c
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Mar 2023 10:31:56.0423
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: DrxkL0E6j1KcaLQXIAGpDcnf22eMKMTjRIYifmGFIhc3MNeA2xT/lc2nNY2Ypvi4VWICJelL7JCnXxAIEMrCmNPFG02DfrIvHhYKO51Lnjc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB8108
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Alex Elder <elder@ieee.org>
Date: Sun, 5 Mar 2023 10:58:19 -0600

> On 2/17/23 7:04 AM, Alex Elder wrote:
>> On 2/17/23 5:57 AM, Alexander Lobakin wrote:
>>>>> just devm_platform_ioremap_resource_byname() be used here for
>>>>> simplicity?
>>>> Previously, virt_raw would be the "real" re-mapped pointer, and then
>>>> virt would be adjusted downward from that.  It was a weird thing to
>>>> do, because the result pointed to a non-mapped address.  But all uses
>>>> of the virt pointer added an offset that was enough to put the result
>>>> into the mapped range.
>>>>
>>>> The new code updates all offsets to account for what the adjustment
>>>> previously did.  The test that got removed isn't necessary any more.
>>> Yeah I got it, just asked that maybe you can now use
>>> platform_ioremap_resource_byname() instead of
>>> platform_get_resource_byname() + ioremap() :)
>>
>> Sorry, I focused on the "devm" part and not this part.
>> Yes I like that, but let me do that as a follow-on
>> patch, and I think I can do it in more than this
>> spot (possibly three, but I have to look closely).
> 
> Looking at this today, the only OF functions that look up a
> resource and I/O remap it in one call are devm_*() variants.
> There is no platform_ioremap_resource_byname() function.
> 
> One that's available is devm_platform_ioremap_resource_byname(),
> which could possibly be used in the two locations that call
> platform_get_resource_byname() followed by ioremap().
> 
> As I said earlier, if I were to use any devm_*() function
> calls the driver, I would want to convert *everything* to
> use devm_*() variants, and I have no plans to do that at
> this time.
> 
> So I will not be implementing your suggestion.

Sure. It's fully up to the developers as it doesn't make the code worse
in any way.

> 
>                     -Alex
> 
>> Thanks.
>>
>>                      -Alex
>>
>>
> 
Thanks,
Olek