Re: [PATCH v3 00/12] i2c: designeware: Add Baikal-T1 System I2C support
From: Andy Shevchenko
Date: Wed May 27 2020 - 09:30:30 EST
On Wed, May 27, 2020 at 12:55:16AM +0300, Serge Semin wrote:
> Jarkko, Wolfram, the merge window is upon us, please review/merge in/whatever
> the patchset.
>
> Initially this has been a small patchset which embedded the Baikal-T1
> System I2C support into the DW APB I2C driver as is by using a simplest
> way. After a short discussion with Andy we decided to implement what he
> suggested (introduce regmap-based accessors and create a glue driver) and
> even more than that to provide some cleanups of the code. So here is what
> this patchset consists of.
>
> First of all we've found out that current implementation of scripts/dtc
> didn't support i2c dt nodes with 10bit and slave flags set in the
> reg property. You'll see an error if you try to dt_binding_check it.
> So the very first patch fixes the problem by adding these flags support
> into the check_i2c_bus_reg() method.
>
> Traditionally we converted the plain text-based DT binding to the DT schema
> and added Baikal-T1 System I2C device support there. This required to mark
> the reg property redundant for Baikal-T1 I2C since its reg-space is
> indirectly accessed by means of the System Controller cmd/read/write
> registers.
>
> Then as Andy suggested we replaced the Synopsys DW APB I2C common driver
> registers IO accessors into the regmap API methods. This doesn't change
> the code logic much, though in two places we managed to replace some bulky
> peaces of code with a ready-to-use regmap methods.
>
> Additionally before adding the glue layer API we initiated a set of cleanups:
> - Define components of the multi-object drivers (like i2c-designware-core.o
> and i2c-designware-paltform.o) with using `-y` suffixed makefile
> variables instead of `-objs` suffixed one. This is encouraged by
> Documentation/kbuild/makefiles.rst text since `-objs` is supposed to be used
> to build host programs.
> - Make DW I2C slave driver depended on the DW I2C core code instead of the
> platform one, which it really is.
> - Move Intel Baytrail semaphore feature to the platform if-clause of the
> kernel config.
>
> After this we finally can introduce the glue layer API for the DW APB I2C
> platform driver. So there are three methods exported from the driver:
> i2c_dw_plat_setup(), i2c_dw_plat_clear(), &i2c_dw_plat_dev_pm_ops to
> setup, cleanup and add PM operations to the glue driven I2C device. Before
> setting the platform DW I2C device up the glue probe code is supposed to
> create an instance of DW I2C device generic object and pre-initialize
> its `struct device` pointer together with optional platform-specific
> flags. In addition to that we converted the MSCC Ocelot SoC I2C specific
> code into the glue layer seeing it's really too specific and, which is more
> important, isn't that complicated so we could unpin it without much of
> worrying to break something.
>
> Meanwhile we discovered that MODEL_CHERRYTRAIL and MODEL_MASK actually
> were no longer used in the code. MODEL_MSCC flag has been discarded since
> the MSCC Ocelot I2C code conversion to the glue driver. So now we can get
> rid of all the MODEL-specific flags.
>
> Finally we introduced a glue driver with Baikal-T1 System I2C device
> support. The driver probe tries to find a syscon regmap, creates the DW
> APB I2C regmap based on it and passes it further to the DW I2C device
> descriptor. Then it does normal DW APB I2C platform setup by calling a
> generic setup method. Cleanup is straightforward. It's just calling a
> generic DW APB I2C clean method.
>
> This patchset is rebased and tested on the mainline Linux kernel 5.6-rc4:
> base-commit: 0e698dfa2822 ("Linux 5.7-rc4")
> tag: v5.7-rc4
>
> Note new vendor prefix for Baikal-T1 System I2C device will be added in
> the framework of the next patchset:
> https://lkml.org/lkml/2020/5/6/1047
> scripts/dtc: check: Add 10bit/slave i2c reg flags support
Hmm... I don't see this patch.
--
With Best Regards,
Andy Shevchenko