Received-SPF: pass (google.com: domain of linux-kernel+bounces-7361-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <caab8bb6-7e14-423b-ad97-7d193e6598bc@amd.com>
Date: Wed, 20 Dec 2023 09:44:36 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/3] mailbox: zynqmp: Move buffered IPI setup to of_match
 selected routine
Content-Language: en-US
To: Michal Simek <michal.simek@amd.com>, jassisinghbrar@gmail.com,
 linux-kernel@vger.kernel.org
Cc: shubhrajyoti.datta@amd.com, tanmay.shah@amd.com,
 linux-arm-kernel@lists.infradead.org
References: <20231214211354.348294-1-ben.levinsky@amd.com>
 <20231214211354.348294-3-ben.levinsky@amd.com>
 <c7ea6fd6-b4cd-45e5-b1b0-12c8e2aa8696@amd.com>
From: Ben Levinsky <ben.levinsky@amd.com>
In-Reply-To: <c7ea6fd6-b4cd-45e5-b1b0-12c8e2aa8696@amd.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?Sk5HdTZMcDdIejZ3eWhiU0FMb2FzNkIxTC9jNGd3aFhUblJMWmZmazVJM3J3?=
 =?utf-8?B?UFR5Kzd5YmtPTXIzbXlpeW96cVVINXBrKzZNMEo1alphVDlhcVR6SEVPVEN2?=
 =?utf-8?B?am5oaWl5NDZURENJdlFCT25RL0FNUDdBU1FvYURJQkpLN29RZER5NzRjeWFk?=
 =?utf-8?B?dmVkdnVRUzlTeU0yTXBJNVZ3ajJYSXcvY1VhbnlWS3grd2xqdmFkc0RobGZJ?=
 =?utf-8?B?K0FyRGZWb3VCMGhNRGtNdlFYaWNVd3p2Tk9NOU5oS0YrNHRsRHllN3daUE81?=
 =?utf-8?B?WERpbFp0eEdIa055R3NNb3pmWDg4eGFBbnB4OFZSMWgwOTRWL2NRM242SUg3?=
 =?utf-8?B?U004QkoxVFB0WTQrMlVLREJobFJhL3JoZTQ2dU53Z0x2OTBEU21sY1R4L1RZ?=
 =?utf-8?B?M3FidnZLckFJWm81SXQ0MmNRZmIzSGJlbnhJQU43U2w4K2VhZjErWndMaVRE?=
 =?utf-8?B?SjNTcTZNSEZuRnMxVDNVb3ljTUtVWHl4M3JZYkV0Tkhsa3JuS1FUT0REMTd3?=
 =?utf-8?B?ckM2dUNSWDNqdUxOZWNFbGNHNW15ZzUzWWpnMi9KZ0d1T3ZaQm10T2Z5Zm9r?=
 =?utf-8?B?NlpxbGlnYi9yVWR4UGs5b0liQ21ZbDNTMG5WUDZGTTFwcDkxNWpDakZySjZh?=
 =?utf-8?B?Uzd0Q3NrNTFYZkhEZStjZjdZQjZIZ25BYzliQUJ6YzF0eENMUnJadUpQUXZX?=
 =?utf-8?B?L1laSVh1MWIwVWZmSjlGa05VUXFJNDlGK21heUxMS2pYd0lvQ1NlVkVOL2hI?=
 =?utf-8?B?N3piQ0NRcG9QZTA1OVFvelJCMkhsMzJvOWE0dXVIOXFaZkRvRys5dXJwMlpK?=
 =?utf-8?B?VmlQYjJkU2x5SnFkSDA2SCt1dXR0aURXYWNCR2RxaERad2tMZHBSOHNFczdB?=
 =?utf-8?B?SFo5RGtYYkpBM1Bmc1ZaL3c1T2JjeG5aT1dpNTZ0eWpLRUxwR0grNTZ5dTdQ?=
 =?utf-8?B?L3A0Zm02MnRZOFlLaFh6eGNNVHc3WVZhemN5S0dFazFaaWh6cTQzTDRldDk1?=
 =?utf-8?B?R2E0QmlGY082R3lXckxNNUZNT0xUY2pFazZONVRmdzhrRGpENEdOU3RFUWQ2?=
 =?utf-8?B?YjJIMFgvd0VIMno2Rmd2UlFSWFZ1U0xiUEpsZkMrMkNoUGVxZzMxd0tHeGE0?=
 =?utf-8?B?NVpRQ0JJMTI0eVh0cXdCUlR0ZTRScXQwQTN2WnRHM01xMmVmOGRSdktOSGVl?=
 =?utf-8?B?V2FFTnUrbjh5bkU1aVRmT2phYkJ5Rjk2QVZueEJYaDVpdUpBUDlzL0JFcGw5?=
 =?utf-8?B?SFdVMVZkN0JVUUsvbStOK0F1V005cVl2c3VxS1lLYzNXOXAxeEdBYUM0UWpT?=
 =?utf-8?B?SGZidVEyQS9XakxZbGR2NWJwK3NKK3ZTUGRsK0lyNHZleG1DZGtaQnJxUlpT?=
 =?utf-8?B?WFAxSTMyTDcra1o0alpCTFp2N2FxTWEzcFphL1MwL3ZlcmlhZUdQODVIbEpm?=
 =?utf-8?B?N0dUaTdVbDNYWkJtNlY3dWtKQjJhT2wzNWpiVjJJSU1iVUNJejVyTk1wUlMz?=
 =?utf-8?B?QTYyRTJTTTE1WDMwWGVEdThST0pTSXJhNnNSME9zOENvV2g1MjlTaFpEZ1d6?=
 =?utf-8?B?RGROR1pwNVVjOVlJeHEveElRekVCTUZyd0JoRTd1U1VmOE5GYWRDU2NTMkRP?=
 =?utf-8?B?QkplcUFndzVKVUtYSU4wVXZpVW9ZTkR0Zis2QmhLMzRTWjZPano4UlFaOW1r?=
 =?utf-8?B?ZHRIRjhWdFFoRUhFWVlva3dOekhSNWt5UnRIZktSeVNEbE1tUGF6bkI3YlYv?=
 =?utf-8?B?eDdxazQ3d095TUVMWlZ0cWtTa2t1T2trRnlxT2Jxc1k2MFI1QnMvb1lsYTVR?=
 =?utf-8?B?Vm9hRlAwcTRwQ0toNkFOT0dvblNqa1BkRjhPYWUxMUVGMng4bHo1MDcwSEVR?=
 =?utf-8?B?S0MyYnVOblRSelBWMU82MjA3d0crR1lud3p4dUgxellmbXhDUlVLdjMwb0Jm?=
 =?utf-8?B?RmlGSkVkSXplUHp1NExkL3NFWlAxZ0dvaDYvSzh4TzBoK3NDWWozcHpqVDZl?=
 =?utf-8?B?dEV6SkJLcU5iYWhsNVl2L204bUFrSHM0b3pVQTBYU2M3U28rN1UxdWJQY1dt?=
 =?utf-8?B?SHdsZVZJLzhNbE5TNFF6djk2THNaTExVRXJXTlFBMUtyeDFucmpWMEJlL3dV?=
 =?utf-8?Q?nHeRB7nrPHsbhoOoc1gLYlclr?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3d0c42e5-27a5-4df1-771b-08dc01835684
X-MS-Exchange-CrossTenant-AuthSource: SA3PR12MB8021.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Dec 2023 17:44:38.4626
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: UTblwNFCzz0urKWYMu8JrDctRaXheT4em3M6uaicZsLbrNEAU+rnJBi2AmCaxxJjjVmEmHBMXxWsP/kBhxq/UQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB7889


On 12/20/23 5:23 AM, Michal Simek wrote:
>
>
> On 12/14/23 22:13, Ben Levinsky wrote:
>> Move routine that initializes the mailboxes for send and receive to
>> a function pointer that is set based on compatible string.
>>
>> Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
>> ---
>>   drivers/mailbox/zynqmp-ipi-mailbox.c | 124 +++++++++++++++++++--------
>>   1 file changed, 89 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
>> index 2c10aa01b3bb..edefb80a6e47 100644
>> --- a/drivers/mailbox/zynqmp-ipi-mailbox.c
>> +++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
>> @@ -72,6 +72,10 @@ struct zynqmp_ipi_mchan {
>>       unsigned int chan_type;
>>   };
>>   +struct zynqmp_ipi_mbox;
>> +
>> +typedef int (*setup_ipi_fn)(struct zynqmp_ipi_mbox *ipi_mbox, struct device_node *node);
>> +
>>   /**
>>    * struct zynqmp_ipi_mbox - Description of a ZynqMP IPI mailbox
>>    *                          platform data.
>> @@ -82,6 +86,7 @@ struct zynqmp_ipi_mchan {
>>    * @mbox:                 mailbox Controller
>>    * @mchans:               array for channels, tx channel and rx channel.
>>    * @irq:                  IPI agent interrupt ID
>> + * setup_ipi_fn:          Function Pointer to set up IPI Channels
>
> Here should be @setup_ipi_fn.
will fix.
>
>>    */
>>   struct zynqmp_ipi_mbox {
>>       struct zynqmp_ipi_pdata *pdata;
>> @@ -89,6 +94,7 @@ struct zynqmp_ipi_mbox {
>>       u32 remote_id;
>>       struct mbox_controller mbox;
>>       struct zynqmp_ipi_mchan mchans[2];
>> +    setup_ipi_fn setup_ipi_fn;
>>   };
>>     /**
>> @@ -466,12 +472,9 @@ static void zynqmp_ipi_mbox_dev_release(struct device *dev)
>>   static int zynqmp_ipi_mbox_probe(struct zynqmp_ipi_mbox *ipi_mbox,
>>                    struct device_node *node)
>>   {
>> -    struct zynqmp_ipi_mchan *mchan;
>>       struct mbox_chan *chans;
>>       struct mbox_controller *mbox;
>> -    struct resource res;
>>       struct device *dev, *mdev;
>> -    const char *name;
>>       int ret;
>>         dev = ipi_mbox->pdata->dev;
>> @@ -491,6 +494,75 @@ static int zynqmp_ipi_mbox_probe(struct zynqmp_ipi_mbox *ipi_mbox,
>>       }
>>       mdev = &ipi_mbox->dev;
>>   +    /* Get the IPI remote agent ID */
>> +    ret = of_property_read_u32(node, "xlnx,ipi-id", &ipi_mbox->remote_id);
>> +    if (ret < 0) {
>> +        dev_err(dev, "No IPI remote ID is specified.\n");
>> +        return ret;
>> +    }
>> +
>> +    ret = ipi_mbox->setup_ipi_fn(ipi_mbox, node);
>> +    if (ret) {
>> +        dev_err(dev, "Failed to set up IPI Buffers.\n");
>> +        return ret;
>> +    }
>> +
>> +    mbox = &ipi_mbox->mbox;
>> +    mbox->dev = mdev;
>> +    mbox->ops = &zynqmp_ipi_chan_ops;
>> +    mbox->num_chans = 2;
>> +    mbox->txdone_irq = false;
>> +    mbox->txdone_poll = true;
>> +    mbox->txpoll_period = 5;
>> +    mbox->of_xlate = zynqmp_ipi_of_xlate;
>> +    chans = devm_kzalloc(mdev, 2 * sizeof(*chans), GFP_KERNEL);
>> +    if (!chans)
>> +        return -ENOMEM;
>> +    mbox->chans = chans;
>> +    chans[IPI_MB_CHNL_TX].con_priv = &ipi_mbox->mchans[IPI_MB_CHNL_TX];
>> +    chans[IPI_MB_CHNL_RX].con_priv = &ipi_mbox->mchans[IPI_MB_CHNL_RX];
>> +    ipi_mbox->mchans[IPI_MB_CHNL_TX].chan_type = IPI_MB_CHNL_TX;
>> +    ipi_mbox->mchans[IPI_MB_CHNL_RX].chan_type = IPI_MB_CHNL_RX;
>> +    ret = devm_mbox_controller_register(mdev, mbox);
>> +    if (ret)
>> +        dev_err(mdev,
>> +            "Failed to register mbox_controller(%d)\n", ret);
>> +    else
>> +        dev_info(mdev,
>> +             "Registered ZynqMP IPI mbox with TX/RX channels.\n");
>> +    return ret;
>> +}
>> +
>> +/**
>> + * zynqmp_ipi_setup - set up IPI Buffers for classic flow
>> + *
>> + * @ipi_mbox: pointer to IPI mailbox private data structure
>> + * @node: IPI mailbox device node
>> + *
>> + * This will be used to set up IPI Buffers for ZynqMP SOC if user
>> + * wishes to use classic driver usage model on new SOC's with only
>> + * buffered IPIs.
>> + *
>> + * Note that bufferless IPIs and mixed usage of buffered and bufferless
>> + * IPIs are not supported with this flow.
>> + *
>> + * This will be invoked with compatible string "xlnx,zynqmp-ipi-mailbox".
>> + *
>> + * Return: 0 for success, negative value for failure
>> + */
>> +static int zynqmp_ipi_setup(struct zynqmp_ipi_mbox *ipi_mbox,
>> +                struct device_node *node)
>> +{
>> +    struct zynqmp_ipi_mchan *mchan;
>> +    struct device *mdev;
>> +    struct resource res;
>> +    struct device *dev;
>
> nit: you can put it to the same line mdev, dev.
will fix.
>
>> +    const char *name;
>> +    int ret;
>> +
>> +    mdev = &ipi_mbox->dev;
>> +    dev = ipi_mbox->pdata->dev;
>> +
>>       mchan = &ipi_mbox->mchans[IPI_MB_CHNL_TX];
>>       name = "local_request_region";
>>       ret = zynqmp_ipi_mbox_get_buf_res(node, name, &res);
>> @@ -565,37 +637,7 @@ static int zynqmp_ipi_mbox_probe(struct zynqmp_ipi_mbox *ipi_mbox,
>>       if (!mchan->rx_buf)
>>           return -ENOMEM;
>>   -    /* Get the IPI remote agent ID */
>> -    ret = of_property_read_u32(node, "xlnx,ipi-id", &ipi_mbox->remote_id);
>> -    if (ret < 0) {
>> -        dev_err(dev, "No IPI remote ID is specified.\n");
>> -        return ret;
>> -    }
>> -
>> -    mbox = &ipi_mbox->mbox;
>> -    mbox->dev = mdev;
>> -    mbox->ops = &zynqmp_ipi_chan_ops;
>> -    mbox->num_chans = 2;
>> -    mbox->txdone_irq = false;
>> -    mbox->txdone_poll = true;
>> -    mbox->txpoll_period = 5;
>> -    mbox->of_xlate = zynqmp_ipi_of_xlate;
>> -    chans = devm_kzalloc(mdev, 2 * sizeof(*chans), GFP_KERNEL);
>> -    if (!chans)
>> -        return -ENOMEM;
>> -    mbox->chans = chans;
>> -    chans[IPI_MB_CHNL_TX].con_priv = &ipi_mbox->mchans[IPI_MB_CHNL_TX];
>> -    chans[IPI_MB_CHNL_RX].con_priv = &ipi_mbox->mchans[IPI_MB_CHNL_RX];
>> -    ipi_mbox->mchans[IPI_MB_CHNL_TX].chan_type = IPI_MB_CHNL_TX;
>> -    ipi_mbox->mchans[IPI_MB_CHNL_RX].chan_type = IPI_MB_CHNL_RX;
>> -    ret = devm_mbox_controller_register(mdev, mbox);
>> -    if (ret)
>> -        dev_err(mdev,
>> -            "Failed to register mbox_controller(%d)\n", ret);
>> -    else
>> -        dev_info(mdev,
>> -             "Registered ZynqMP IPI mbox with TX/RX channels.\n");
>> -    return ret;
>> +    return 0;
>>   }
>>     /**
>> @@ -626,6 +668,7 @@ static int zynqmp_ipi_probe(struct platform_device *pdev)
>>       struct zynqmp_ipi_pdata *pdata;
>>       struct zynqmp_ipi_mbox *mbox;
>>       int num_mboxes, ret = -EINVAL;
>> +    setup_ipi_fn ipi_fn;
>>         num_mboxes = of_get_available_child_count(np);
>>       if (num_mboxes == 0) {
>> @@ -646,9 +689,18 @@ static int zynqmp_ipi_probe(struct platform_device *pdev)
>>           return ret;
>>       }
>>   +    ipi_fn = (setup_ipi_fn)device_get_match_data(&pdev->dev);
>> +    if (!ipi_fn) {
>> +        dev_err(dev,
>> +            "Mbox Compatible String is missing IPI Setup fn.\n");
>> +        return -ENODEV;
>> +    }
>> +
>>       pdata->num_mboxes = num_mboxes;
>>         mbox = pdata->ipi_mboxes;
>> +    mbox->setup_ipi_fn = ipi_fn;
>> +
>>       for_each_available_child_of_node(np, nc) {
>>           mbox->pdata = pdata;
>>           ret = zynqmp_ipi_mbox_probe(mbox, nc);
>> @@ -694,7 +746,9 @@ static int zynqmp_ipi_remove(struct platform_device *pdev)
>>   }
>>     static const struct of_device_id zynqmp_ipi_of_match[] = {
>> -    { .compatible = "xlnx,zynqmp-ipi-mailbox" },
>> +    { .compatible = "xlnx,zynqmp-ipi-mailbox",
>> +      .data = &zynqmp_ipi_setup,
>> +    },
>>       {},
>>   };
>>   MODULE_DEVICE_TABLE(of, zynqmp_ipi_of_match);
>
>
> M