Messages in this thread | | | From | Andy Shevchenko <> | Date | Thu, 28 May 2020 16:51:03 +0300 | Subject | Re: [PATCH v3 4/6] bus: Add Baikal-T1 AXI-bus driver |
| |
On Thu, May 28, 2020 at 4:40 PM Serge Semin <Sergey.Semin@baikalelectronics.ru> wrote: > > On Thu, May 28, 2020 at 01:00:57AM +0300, Andy Shevchenko wrote: > > On Tuesday, May 26, 2020, Serge Semin <Sergey.Semin@baikalelectronics.ru> > > wrote: > > > > > AXI3-bus is the main communication bus connecting all high-speed > > > peripheral IP-cores with RAM controller and MIPS P5600 cores on Baikal-T1 > > > SoC. Bus traffic arbitration is done by means of DW AMBA 3 AXI > > > Interconnect (so called AXI Main Interconnect) routing IO requests from > > > one SoC block to another. This driver provides a way to detect any bus > > > protocol errors and device not responding situations by means of an > > > embedded on top of the interconnect errors handler block (EHB). AXI > > > Interconnect QoS arbitration tuning is currently unsupported. > > > The bus doesn't provide a way to detect the interconnected devices, > > > so they are supposed to be statically defined like by means of the > > > simple-bus sub-nodes. > > > > > > > > Few comments in case if you need a new version. Main point is about > > sysfs_streq(). > > Hello, Andy. Thanks for your comments. I'll address most of them in a follow-up > patches. See just one note below. > > > > > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> > > > Cc: Paul Burton <paulburton@kernel.org> > > > Cc: Olof Johansson <olof@lixom.net> > > > Cc: Rob Herring <robh+dt@kernel.org> > > > Cc: linux-mips@vger.kernel.org > > > Cc: soc@kernel.org > > > Cc: devicetree@vger.kernel.org > > > > > [nip] > > > > + > > > +static void bt1_axi_clear_data(void *data) > > > +{ > > > + struct bt1_axi *axi = data; > > > + struct platform_device *pdev = to_platform_device(axi->dev); > > > + > > > + platform_set_drvdata(pdev, NULL); > > > > > > > Doesn't device driver core do this already? > > It doesn't on remove.
__device_release_driver() calls dev_set_drvdata(dev, NULL); What did I miss?
> This cleanups the drvdata pointer when the driver is > unloaded at the moment of remove() callback calling. This is a good > practice to leave the device the same as it has been before usage including > the pointer value. In this case if theoretically someone (though very > unlikely, but anyway) would try to use the pointer without having it > initialized, the NULL-dereference would pop up, otherwise we may > corrupt someones memory, which is very nasty.
The above is right and I agree with.
-- With Best Regards, Andy Shevchenko
| |