| Message ID | 20220511231810.4928-22-Sergey.Semin@baikalelectronics.ru |
|---|---|
| State | New |
| Headers | show |
| Series | ata: ahci: Add DWC/Baikal-T1 AHCI SATA support | expand |
On 5/12/22 01:18, Serge Semin wrote: > Some DWC AHCI SATA IP-core derivatives require to perform small platform > or IP-core specific setups. They are too small to be placed in a dedicated > driver. It's just much easier to have a set of quirks for them right in > the DWC AHCI driver code. Since we are about to add such platform support, > as a pre-requisite we introduce a platform-data based DWC AHCI quirks API. > The platform data can be used to define the flags passed to the > ahci_platform_get_resources() method, additional AHCI host-flags and a set > of callbacks to initialize, re-initialize and clear the platform settings. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Changelog v2: > - Change the local objects prefix from 'dwc_ahci_' to 'ahci_dwc_'. > (@Damien) > --- > drivers/ata/ahci_dwc.c | 52 ++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 48 insertions(+), 4 deletions(-) > This really could be merged with patch 19; as you're adding a new driver you might as well fold this patch in to avoid further modifications. Cheers, Hannes
On Thu, May 12, 2022 at 09:12:46AM +0200, Hannes Reinecke wrote: > On 5/12/22 01:18, Serge Semin wrote: > > Some DWC AHCI SATA IP-core derivatives require to perform small platform > > or IP-core specific setups. They are too small to be placed in a dedicated > > driver. It's just much easier to have a set of quirks for them right in > > the DWC AHCI driver code. Since we are about to add such platform support, > > as a pre-requisite we introduce a platform-data based DWC AHCI quirks API. > > The platform data can be used to define the flags passed to the > > ahci_platform_get_resources() method, additional AHCI host-flags and a set > > of callbacks to initialize, re-initialize and clear the platform settings. > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > --- > > > > Changelog v2: > > - Change the local objects prefix from 'dwc_ahci_' to 'ahci_dwc_'. > > (@Damien) > > --- > > drivers/ata/ahci_dwc.c | 52 ++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 48 insertions(+), 4 deletions(-) > > > This really could be merged with patch 19; as you're adding a new driver you > might as well fold this patch in to avoid further modifications. That merely depends on the development approach. I saw and reviewed not one kernel patchsets introducing new drivers and adding features one after another. Moreover there is no kernel patches requirement which would constitutes not to do so. In this particular case the modification isn't complicated at all thus can be reviewed as is. -Sergey > > Cheers, > > Hannes > -- > Dr. Hannes Reinecke Kernel Storage Architect > hare@suse.de +49 911 74053 688 > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg > HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
Hi Serge,
I love your patch! Perhaps something to improve:
[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on robh/for-next linus/master v5.18-rc6 next-20220513]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Serge-Semin/ata-ahci-Add-DWC-Baikal-T1-AHCI-SATA-support/20220512-072125
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: x86_64-rhel-8.3-kselftests (https://download.01.org/0day-ci/archive/20220514/202205140748.Lb7DqbPf-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/intel-lab-lkp/linux/commit/bc74e5ca4cbfd2bef25a9ecefc401e7d1af3df43
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Serge-Semin/ata-ahci-Add-DWC-Baikal-T1-AHCI-SATA-support/20220512-072125
git checkout bc74e5ca4cbfd2bef25a9ecefc401e7d1af3df43
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/ata/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/ata/ahci_dwc.c:414:27: sparse: sparse: symbol 'ahci_dwc_plat' was not declared. Should it be static?
Please review and possibly fold the followup patch.
diff --git a/drivers/ata/ahci_dwc.c b/drivers/ata/ahci_dwc.c index 73380aaaebab..f987efa1ad59 100644 --- a/drivers/ata/ahci_dwc.c +++ b/drivers/ata/ahci_dwc.c @@ -90,7 +90,16 @@ #define AHCI_DWC_PORT_PHYCR 0x74 #define AHCI_DWC_PORT_PHYSR 0x78 +struct ahci_dwc_plat_data { + unsigned int pflags; + unsigned int hflags; + int (*init)(struct ahci_host_priv *hpriv); + int (*reinit)(struct ahci_host_priv *hpriv); + void (*clear)(struct ahci_host_priv *hpriv); +}; + struct ahci_dwc_host_priv { + const struct ahci_dwc_plat_data *pdata; struct platform_device *pdev; u32 timv; @@ -107,11 +116,15 @@ static struct ahci_host_priv *ahci_dwc_get_resources(struct platform_device *pde return ERR_PTR(-ENOMEM); dpriv->pdev = pdev; + dpriv->pdata = device_get_match_data(&pdev->dev); + if (!dpriv->pdata) + return ERR_PTR(-EINVAL); - hpriv = ahci_platform_get_resources(pdev, AHCI_PLATFORM_GET_RESETS); + hpriv = ahci_platform_get_resources(pdev, dpriv->pdata->pflags); if (IS_ERR(hpriv)) return hpriv; + hpriv->flags |= dpriv->pdata->hflags; hpriv->plat_data = (void *)dpriv; return hpriv; @@ -242,22 +255,33 @@ static int ahci_dwc_init_dmacr(struct ahci_host_priv *hpriv) static int ahci_dwc_init_host(struct ahci_host_priv *hpriv) { + struct ahci_dwc_host_priv *dpriv = hpriv->plat_data; int rc; rc = ahci_platform_enable_resources(hpriv); if (rc) return rc; + if (dpriv->pdata->init) { + rc = dpriv->pdata->init(hpriv); + if (rc) + goto err_disable_resources; + } + ahci_dwc_check_cap(hpriv); ahci_dwc_init_timer(hpriv); rc = ahci_dwc_init_dmacr(hpriv); if (rc) - goto err_disable_resources; + goto err_clear_platform; return 0; +err_clear_platform: + if (dpriv->pdata->clear) + dpriv->pdata->clear(hpriv); + err_disable_resources: ahci_platform_disable_resources(hpriv); @@ -275,6 +299,12 @@ static int ahci_dwc_reinit_host(struct ahci_host_priv *hpriv) if (rc) return rc; + if (dpriv->pdata->reinit) { + rc = dpriv->pdata->reinit(hpriv); + if (rc) + goto err_disable_resources; + } + writel(dpriv->timv, hpriv->mmio + AHCI_DWC_HOST_TIMER1MS); for_each_set_bit(i, &port_map, AHCI_MAX_PORTS) { @@ -283,10 +313,20 @@ static int ahci_dwc_reinit_host(struct ahci_host_priv *hpriv) } return 0; + +err_disable_resources: + ahci_platform_disable_resources(hpriv); + + return rc; } static void ahci_dwc_clear_host(struct ahci_host_priv *hpriv) { + struct ahci_dwc_host_priv *dpriv = hpriv->plat_data; + + if (dpriv->pdata->clear) + dpriv->pdata->clear(hpriv); + ahci_platform_disable_resources(hpriv); } @@ -371,9 +411,13 @@ static int ahci_dwc_resume(struct device *dev) static SIMPLE_DEV_PM_OPS(ahci_dwc_pm_ops, ahci_dwc_suspend, ahci_dwc_resume); +struct ahci_dwc_plat_data ahci_dwc_plat = { + .pflags = AHCI_PLATFORM_GET_RESETS, +}; + static const struct of_device_id ahci_dwc_of_match[] = { - { .compatible = "snps,dwc-ahci", }, - { .compatible = "snps,spear-ahci", }, + { .compatible = "snps,dwc-ahci", &ahci_dwc_plat }, + { .compatible = "snps,spear-ahci", &ahci_dwc_plat }, {}, }; MODULE_DEVICE_TABLE(of, ahci_dwc_of_match);
Some DWC AHCI SATA IP-core derivatives require to perform small platform or IP-core specific setups. They are too small to be placed in a dedicated driver. It's just much easier to have a set of quirks for them right in the DWC AHCI driver code. Since we are about to add such platform support, as a pre-requisite we introduce a platform-data based DWC AHCI quirks API. The platform data can be used to define the flags passed to the ahci_platform_get_resources() method, additional AHCI host-flags and a set of callbacks to initialize, re-initialize and clear the platform settings. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> --- Changelog v2: - Change the local objects prefix from 'dwc_ahci_' to 'ahci_dwc_'. (@Damien) --- drivers/ata/ahci_dwc.c | 52 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 4 deletions(-)