On 7/19/20 9:47 PM, Joel Stanley wrote:
> On Sun, 19 Jul 2020 at 22:13, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>>
>> On Fri, May 01, 2020 at 10:08:32AM -0500, Eddie James wrote:
>>> The P10 OCC has a different SRAM address for the command and response
>>> buffers. In addition, the SBE commands to access the SRAM have changed
>>> format. Add versioning to the driver to handle these differences.
>>>
>>> Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx>
>>
>> I don't recall seeing a maintainer Ack for this patch, nor any response
>> at all. I'd be happy to apply the patch through hwmon, but I would need
>> an Ack from a maintainer.
>
> That would be great. I had one question before it goes in, but once
> Eddie has sorted that out it can go through your tree.
>
>>
>> Thanks,
>> Guenter
>>
>>> ---
>>> drivers/fsi/fsi-occ.c | 126 ++++++++++++++++++++++++++++++------------
>>> 1 file changed, 92 insertions(+), 34 deletions(-)
>
>>> @@ -508,6 +557,7 @@ static int occ_probe(struct platform_device *pdev)
>>> struct occ *occ;
>>> struct platform_device *hwmon_dev;
>>> struct device *dev = &pdev->dev;
>>> + const void *md = of_device_get_match_data(dev);
>>> struct platform_device_info hwmon_dev_info = {
>>> .parent = dev,
>>> .name = "occ-hwmon",
>>> @@ -517,6 +567,7 @@ static int occ_probe(struct platform_device *pdev)
>>> if (!occ)
>>> return -ENOMEM;
>>>
>>> + occ->version = (enum versions)md;
>
> The 0day bot warns about this when bulding for 64 bit architectures.
>
> How about you drop the match data and instead match on the compatible
> string to know which version to expect?
>
That seems to be less desirable and defeat the purpose of of_device_get_match_data().
I have seen better solutions. Some options:
version = (uintptr_t)of_device_get_match_data(dev);
version = (unsigned long)of_device_get_match_data(dev);
version = (enum versions)(uintptr_t)of_device_get_match_data(dev);
You don't otherwise use the "md" variable, so you might as well drop it.
Guenter
>>> occ->dev = dev;
>>> occ->sbefifo = dev->parent;
>>> mutex_init(&occ->occ_lock);
>>> @@ -575,7 +626,14 @@ static int occ_remove(struct platform_device *pdev)
>>> }
>>>
>>> static const struct of_device_id occ_match[] = {
>>> - { .compatible = "ibm,p9-occ" },
>>> + {
>>> + .compatible = "ibm,p9-occ",
>>> + .data = (void *)occ_p9
>>> + },
>>> + {
>>> + .compatible = "ibm,p10-occ",
>>> + .data = (void *)occ_p10
>>> + },
>>> { },
>>> };
>>>
