On Tue, 24 Nov 2020 at 02:02, Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx> wrote:
>
> Hi Loic,
> On 2020-11-23 06:11 AM, Loic Poulain wrote:
> > Add support for resetting the device, reset can be triggered in case
> > of error or manually via sysfs (/sys/bus/pci/devices/*/reset).
> >
> > Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxx>
> > ---
> > drivers/bus/mhi/pci_generic.c | 117
> > +++++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 104 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/bus/mhi/pci_generic.c
> > b/drivers/bus/mhi/pci_generic.c
> > index 0c07cf5..b48c382 100644
> > --- a/drivers/bus/mhi/pci_generic.c
> > +++ b/drivers/bus/mhi/pci_generic.c
> > @@ -8,6 +8,7 @@
> > * Copyright (C) 2020 Linaro Ltd <loic.poulain@xxxxxxxxxx>
> > */
> >
> > +#include <linux/delay.h>
> > #include <linux/device.h>
> > #include <linux/mhi.h>
> > #include <linux/module.h>
> > @@ -179,6 +180,16 @@ static const struct pci_device_id
> > mhi_pci_id_table[] = {
> > };
> > MODULE_DEVICE_TABLE(pci, mhi_pci_id_table);
> >
> > +enum mhi_pci_device_status {
> > + MHI_PCI_DEV_STARTED,
> > +};
> > +
> > +struct mhi_pci_device {
> > + struct mhi_controller mhi_cntrl;
> > + struct pci_saved_state *pci_state;
> > + unsigned long status;
> > +};
> > +
> > static int mhi_pci_read_reg(struct mhi_controller *mhi_cntrl,
> > void __iomem *addr, u32 *out)
> > {
> > @@ -203,6 +214,20 @@ static inline void mhi_pci_reset(struct
> > mhi_controller *mhi_cntrl)
> > writel(1, mhi_cntrl->regs + DEV_RESET_REG);
> > }
> >
> > +static bool mhi_pci_is_alive(struct mhi_controller *mhi_cntrl)
> > +{
> > + struct pci_dev *pdev = to_pci_dev(mhi_cntrl->cntrl_dev);
> > + u16 vendor = 0;
> > +
> > + if (pci_read_config_word(pdev, PCI_VENDOR_ID, &vendor))
> > + return false;
> > +
> > + if (vendor == (u16) ~0 || vendor == 0)
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > static int mhi_pci_claim(struct mhi_controller *mhi_cntrl,
> > unsigned int bar_num, u64 dma_mask)
> > {
> > @@ -298,16 +323,18 @@ static int mhi_pci_probe(struct pci_dev *pdev,
> > const struct pci_device_id *id)
> > {
> > const struct mhi_pci_dev_info *info = (struct mhi_pci_dev_info *)
> > id->driver_data;
> > const struct mhi_controller_config *mhi_cntrl_config;
> > + struct mhi_pci_device *mhi_pdev;
> > struct mhi_controller *mhi_cntrl;
> > int err;
> >
> > dev_dbg(&pdev->dev, "MHI PCI device found: %s\n", info->name);
> >
> > - mhi_cntrl = mhi_alloc_controller();
> > - if (!mhi_cntrl)
> > + mhi_pdev = devm_kzalloc(&pdev->dev, sizeof(*mhi_pdev), GFP_KERNEL);
> > + if (!mhi_pdev)
> > return -ENOMEM;
> Were you able to look in to using alloc/free APIs? We do want to mandate
> these for any controller as they would read garbage serial/OEM PK HASH
> values
> if they were to not abide by it and end up using kmalloc instead of
> kzalloc.
>
> We understand you're using kzalloc but it still deviates from furthering
> our
> recommendations of using exported APIs.
OK, understood, I need to create a mhi_initialize_controller() then,
to initialize structure for non dynamically allocated controller and
non-standalone controller structures. will do in v3.
Regards,
Loic
