* Re: [PATCH 1/2] i2c: aspeed: added driver for Aspeed I2C
From: kbuild test robot @ 2016-09-10 0:48 UTC (permalink / raw)
Cc: kbuild-all-JC7UmRfGjtg, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, joel-U3u1mxZcP9KHXe+LvDLADg,
jk-mnsaURCQ41sdnm+yROfE0A, Brendan Higgins
In-Reply-To: <1473450870-10333-2-git-send-email-brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 6646 bytes --]
Hi Brendan,
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v4.8-rc5 next-20160909]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Brendan-Higgins/i2c-aspeed-added-driver-for-Aspeed-I2C/20160910-035912
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: blackfin-allmodconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 4.6.3
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin
All errors (new ones prefixed by >>):
drivers/i2c/busses/i2c-aspeed.c:697:1: error: 'aspeed_i2c_of_table' undeclared here (not in a function)
>> drivers/i2c/busses/i2c-aspeed.c:812:1: error: redefinition of '__inittest'
drivers/i2c/busses/i2c-aspeed.c:707:1: note: previous definition of '__inittest' was here
>> drivers/i2c/busses/i2c-aspeed.c:812:1: error: redefinition of 'init_module'
drivers/i2c/busses/i2c-aspeed.c:707:1: note: previous definition of 'init_module' was here
>> drivers/i2c/busses/i2c-aspeed.c:812:1: error: redefinition of '__exittest'
drivers/i2c/busses/i2c-aspeed.c:707:1: note: previous definition of '__exittest' was here
>> drivers/i2c/busses/i2c-aspeed.c:812:1: error: redefinition of 'cleanup_module'
drivers/i2c/busses/i2c-aspeed.c:707:1: note: previous definition of 'cleanup_module' was here
>> drivers/i2c/busses/i2c-aspeed.c:801:1: error: '__mod_of__aspeed_i2c_of_table_device_table' aliased to undefined symbol 'aspeed_i2c_of_table'
>> drivers/i2c/busses/i2c-aspeed.c:801:1: error: '__mod_of__aspeed_i2c_of_table_device_table' aliased to undefined symbol 'aspeed_i2c_of_table'
vim +/__inittest +812 drivers/i2c/busses/i2c-aspeed.c
691
692 static const struct of_device_id aspeed_i2c_bus_of_table[] = {
693 { .compatible = "aspeed,ast2400-i2c-bus", },
694 { .compatible = "aspeed,ast2500-i2c-bus", },
695 { },
696 };
> 697 MODULE_DEVICE_TABLE(of, aspeed_i2c_of_table);
698
699 static struct platform_driver aspeed_i2c_bus_driver = {
700 .probe = aspeed_i2c_probe_bus,
701 .remove = aspeed_i2c_remove_bus,
702 .driver = {
703 .name = "ast-i2c-bus",
704 .of_match_table = aspeed_i2c_bus_of_table,
705 },
706 };
707 module_platform_driver(aspeed_i2c_bus_driver);
708
709 static void aspeed_i2c_controller_irq(struct irq_desc *desc)
710 {
711 struct aspeed_i2c_controller *c = irq_desc_get_handler_data(desc);
712 unsigned long p, status;
713 unsigned int bus_irq;
714
715 status = readl(c->base);
716 for_each_set_bit(p, &status, ASPEED_I2C_NUM_BUS) {
717 bus_irq = irq_find_mapping(c->irq_domain, p);
718 generic_handle_irq(bus_irq);
719 }
720 }
721
722 static int aspeed_i2c_probe_controller(struct platform_device *pdev)
723 {
724 struct aspeed_i2c_controller *controller;
725 struct device_node *np;
726 struct resource *res;
727
728 controller = kzalloc(sizeof(*controller), GFP_KERNEL);
729 if (!controller)
730 return -ENOMEM;
731
732 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
733 controller->base = devm_ioremap_resource(&pdev->dev, res);
734 if (IS_ERR(controller->base))
735 return PTR_ERR(controller->base);
736
737 controller->irq = platform_get_irq(pdev, 0);
738 if (controller->irq < 0) {
739 dev_err(&pdev->dev, "no platform IRQ\n");
740 return -ENXIO;
741 }
742
743 controller->irq_domain = irq_domain_add_linear(pdev->dev.of_node,
744 ASPEED_I2C_NUM_BUS, &irq_domain_simple_ops, NULL);
745 if (!controller->irq_domain) {
746 dev_err(&pdev->dev, "no IRQ domain\n");
747 return -ENXIO;
748 }
749 controller->irq_domain->name = "ast-i2c-domain";
750
751 irq_set_chained_handler_and_data(controller->irq,
752 aspeed_i2c_controller_irq, controller);
753
754 controller->dev = &pdev->dev;
755
756 platform_set_drvdata(pdev, controller);
757
758 dev_info(controller->dev, "i2c controller registered, irq %d\n",
759 controller->irq);
760
761 for_each_child_of_node(pdev->dev.of_node, np) {
762 int ret;
763 u32 bus_num;
764 char bus_id[sizeof("i2c-12345")];
765
766 /*
767 * Set a useful name derived from the bus number; the device
768 * tree should provide us with one that corresponds to the
769 * hardware numbering. If the property is missing the
770 * probe would fail so just skip it here.
771 */
772
773 ret = of_property_read_u32(np, "bus", &bus_num);
774 if (ret)
775 continue;
776
777 ret = snprintf(bus_id, sizeof(bus_id), "i2c-%u", bus_num);
778 if (ret >= sizeof(bus_id))
779 continue;
780
781 of_platform_device_create(np, bus_id, &pdev->dev);
782 of_node_put(np);
783 }
784
785 return 0;
786 }
787
788 static int aspeed_i2c_remove_controller(struct platform_device *pdev)
789 {
790 struct aspeed_i2c_controller *controller = platform_get_drvdata(pdev);
791
792 irq_domain_remove(controller->irq_domain);
793 return 0;
794 }
795
796 static const struct of_device_id aspeed_i2c_controller_of_table[] = {
797 { .compatible = "aspeed,ast2400-i2c-controller", },
798 { .compatible = "aspeed,ast2500-i2c-controller", },
799 { },
800 };
> 801 MODULE_DEVICE_TABLE(of, aspeed_i2c_of_table);
802
803 static struct platform_driver aspeed_i2c_controller_driver = {
804 .probe = aspeed_i2c_probe_controller,
805 .remove = aspeed_i2c_remove_controller,
806 .driver = {
807 .name = "ast-i2c-controller",
808 .of_match_table = aspeed_i2c_controller_of_table,
809 },
810 };
811
> 812 module_platform_driver(aspeed_i2c_controller_driver);
813
814 MODULE_AUTHOR("Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>");
815 MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 41250 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] i2c: aspeed: added driver for Aspeed I2C
From: kbuild test robot @ 2016-09-09 22:48 UTC (permalink / raw)
Cc: kbuild-all-JC7UmRfGjtg, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, joel-U3u1mxZcP9KHXe+LvDLADg,
jk-mnsaURCQ41sdnm+yROfE0A, Brendan Higgins
In-Reply-To: <1473450870-10333-2-git-send-email-brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 11545 bytes --]
Hi Brendan,
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v4.8-rc5 next-20160909]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Brendan-Higgins/i2c-aspeed-added-driver-for-Aspeed-I2C/20160910-035912
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
In file included from drivers/i2c/busses/i2c-aspeed.c:14:0:
>> drivers/i2c/busses/i2c-aspeed.c:697:25: error: 'aspeed_i2c_of_table' undeclared here (not in a function)
MODULE_DEVICE_TABLE(of, aspeed_i2c_of_table);
^
include/linux/module.h:213:21: note: in definition of macro 'MODULE_DEVICE_TABLE'
extern const typeof(name) __mod_##type##__##name##_device_table \
^~~~
>> include/linux/module.h:130:27: error: redefinition of '__inittest'
static inline initcall_t __inittest(void) \
^
include/linux/device.h:1350:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
>> include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-aspeed.c:812:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(aspeed_i2c_controller_driver);
^~~~~~~~~~~~~~~~~~~~~~
include/linux/module.h:130:27: note: previous definition of '__inittest' was here
static inline initcall_t __inittest(void) \
^
include/linux/device.h:1350:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
>> include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/i2c/busses/i2c-aspeed.c:707:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(aspeed_i2c_bus_driver);
^~~~~~~~~~~~~~~~~~~~~~
>> include/linux/module.h:132:6: error: redefinition of 'init_module'
int init_module(void) __attribute__((alias(#initfn)));
^
include/linux/device.h:1350:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
>> include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-aspeed.c:812:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(aspeed_i2c_controller_driver);
^~~~~~~~~~~~~~~~~~~~~~
include/linux/module.h:132:6: note: previous definition of 'init_module' was here
int init_module(void) __attribute__((alias(#initfn)));
^
include/linux/device.h:1350:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
>> include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/i2c/busses/i2c-aspeed.c:707:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(aspeed_i2c_bus_driver);
^~~~~~~~~~~~~~~~~~~~~~
>> include/linux/module.h:136:27: error: redefinition of '__exittest'
static inline exitcall_t __exittest(void) \
^
include/linux/device.h:1355:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
>> include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-aspeed.c:812:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(aspeed_i2c_controller_driver);
^~~~~~~~~~~~~~~~~~~~~~
include/linux/module.h:136:27: note: previous definition of '__exittest' was here
static inline exitcall_t __exittest(void) \
^
include/linux/device.h:1355:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
>> include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/i2c/busses/i2c-aspeed.c:707:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(aspeed_i2c_bus_driver);
^~~~~~~~~~~~~~~~~~~~~~
>> include/linux/module.h:138:7: error: redefinition of 'cleanup_module'
void cleanup_module(void) __attribute__((alias(#exitfn)));
^
include/linux/device.h:1355:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
>> include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-aspeed.c:812:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(aspeed_i2c_controller_driver);
^~~~~~~~~~~~~~~~~~~~~~
include/linux/module.h:138:7: note: previous definition of 'cleanup_module' was here
void cleanup_module(void) __attribute__((alias(#exitfn)));
^
include/linux/device.h:1355:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
>> include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/i2c/busses/i2c-aspeed.c:707:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(aspeed_i2c_bus_driver);
^~~~~~~~~~~~~~~~~~~~~~
>> include/linux/module.h:213:27: error: '__mod_of__aspeed_i2c_of_table_device_table' aliased to undefined symbol 'aspeed_i2c_of_table'
extern const typeof(name) __mod_##type##__##name##_device_table \
^
>> drivers/i2c/busses/i2c-aspeed.c:801:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
MODULE_DEVICE_TABLE(of, aspeed_i2c_of_table);
^~~~~~~~~~~~~~~~~~~
>> include/linux/module.h:213:27: error: '__mod_of__aspeed_i2c_of_table_device_table' aliased to undefined symbol 'aspeed_i2c_of_table'
extern const typeof(name) __mod_##type##__##name##_device_table \
^
vim +/aspeed_i2c_of_table +697 drivers/i2c/busses/i2c-aspeed.c
691
692 static const struct of_device_id aspeed_i2c_bus_of_table[] = {
693 { .compatible = "aspeed,ast2400-i2c-bus", },
694 { .compatible = "aspeed,ast2500-i2c-bus", },
695 { },
696 };
> 697 MODULE_DEVICE_TABLE(of, aspeed_i2c_of_table);
698
699 static struct platform_driver aspeed_i2c_bus_driver = {
700 .probe = aspeed_i2c_probe_bus,
701 .remove = aspeed_i2c_remove_bus,
702 .driver = {
703 .name = "ast-i2c-bus",
704 .of_match_table = aspeed_i2c_bus_of_table,
705 },
706 };
> 707 module_platform_driver(aspeed_i2c_bus_driver);
708
709 static void aspeed_i2c_controller_irq(struct irq_desc *desc)
710 {
711 struct aspeed_i2c_controller *c = irq_desc_get_handler_data(desc);
712 unsigned long p, status;
713 unsigned int bus_irq;
714
715 status = readl(c->base);
716 for_each_set_bit(p, &status, ASPEED_I2C_NUM_BUS) {
717 bus_irq = irq_find_mapping(c->irq_domain, p);
718 generic_handle_irq(bus_irq);
719 }
720 }
721
722 static int aspeed_i2c_probe_controller(struct platform_device *pdev)
723 {
724 struct aspeed_i2c_controller *controller;
725 struct device_node *np;
726 struct resource *res;
727
728 controller = kzalloc(sizeof(*controller), GFP_KERNEL);
729 if (!controller)
730 return -ENOMEM;
731
732 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
733 controller->base = devm_ioremap_resource(&pdev->dev, res);
734 if (IS_ERR(controller->base))
735 return PTR_ERR(controller->base);
736
737 controller->irq = platform_get_irq(pdev, 0);
738 if (controller->irq < 0) {
739 dev_err(&pdev->dev, "no platform IRQ\n");
740 return -ENXIO;
741 }
742
743 controller->irq_domain = irq_domain_add_linear(pdev->dev.of_node,
744 ASPEED_I2C_NUM_BUS, &irq_domain_simple_ops, NULL);
745 if (!controller->irq_domain) {
746 dev_err(&pdev->dev, "no IRQ domain\n");
747 return -ENXIO;
748 }
749 controller->irq_domain->name = "ast-i2c-domain";
750
751 irq_set_chained_handler_and_data(controller->irq,
752 aspeed_i2c_controller_irq, controller);
753
754 controller->dev = &pdev->dev;
755
756 platform_set_drvdata(pdev, controller);
757
758 dev_info(controller->dev, "i2c controller registered, irq %d\n",
759 controller->irq);
760
761 for_each_child_of_node(pdev->dev.of_node, np) {
762 int ret;
763 u32 bus_num;
764 char bus_id[sizeof("i2c-12345")];
765
766 /*
767 * Set a useful name derived from the bus number; the device
768 * tree should provide us with one that corresponds to the
769 * hardware numbering. If the property is missing the
770 * probe would fail so just skip it here.
771 */
772
773 ret = of_property_read_u32(np, "bus", &bus_num);
774 if (ret)
775 continue;
776
777 ret = snprintf(bus_id, sizeof(bus_id), "i2c-%u", bus_num);
778 if (ret >= sizeof(bus_id))
779 continue;
780
781 of_platform_device_create(np, bus_id, &pdev->dev);
782 of_node_put(np);
783 }
784
785 return 0;
786 }
787
788 static int aspeed_i2c_remove_controller(struct platform_device *pdev)
789 {
790 struct aspeed_i2c_controller *controller = platform_get_drvdata(pdev);
791
792 irq_domain_remove(controller->irq_domain);
793 return 0;
794 }
795
796 static const struct of_device_id aspeed_i2c_controller_of_table[] = {
797 { .compatible = "aspeed,ast2400-i2c-controller", },
798 { .compatible = "aspeed,ast2500-i2c-controller", },
799 { },
800 };
> 801 MODULE_DEVICE_TABLE(of, aspeed_i2c_of_table);
802
803 static struct platform_driver aspeed_i2c_controller_driver = {
804 .probe = aspeed_i2c_probe_controller,
805 .remove = aspeed_i2c_remove_controller,
806 .driver = {
807 .name = "ast-i2c-controller",
808 .of_match_table = aspeed_i2c_controller_of_table,
809 },
810 };
811
> 812 module_platform_driver(aspeed_i2c_controller_driver);
813
814 MODULE_AUTHOR("Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>");
815 MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 55598 bytes --]
^ permalink raw reply
* Re: [PATCH v5] i2c: imx: make bus recovery through pinctrl optional
From: Uwe Kleine-König @ 2016-09-09 20:59 UTC (permalink / raw)
To: Stefan Agner
Cc: Leo Li, Gao Pan, Wolfram Sang, lkml, Li Yang, linux-gpio,
linux-i2c, linux-arm-kernel
In-Reply-To: <649e7e70becb94adf3b8c4ae2409ea00@agner.ch>
Hello,
On Fri, Sep 09, 2016 at 01:34:31PM -0700, Stefan Agner wrote:
> Yeah it is a bit a wording thing: In my understanding, pinctrl is
> required on SoC's witch have a pin controller... It is just that the
> driver does not need to get the pinctrl by itself because the stack is
> taking care of it implicitly. And yes, that makes the particular example
> not a real world example.
At first I thought, too, that it's a fatal problem if getting the
pinctrl stuff fails. IMHO that shows that the comments (or the code) are
still not good enough.
Maybe we should do something like that:
/*
* As the IP doesn't support bus recovery, we have to switch SCL and SDA
* to their GPIO function and do some bitbanging. These alternative
* pinmux settings can be described in the device tree by a separate
* pinctrl state "gpio". If this is missing this is not a big problem,
* the only implication is that we can't do bus recovery.
*/
static void i2c_imx_init_recovery_info(...)
{
...
and then put
i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
if (IS_ERR(i2c_imx->pinctrl))
return;
into this function (and remove it from i2c_imx_probe). This makes it
more obvious that .pinctrl is only ever used for recovery and as
i2c_imx_init_recovery_info is void there is no error to propagate.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use.
From: Peter Rosin @ 2016-09-09 20:40 UTC (permalink / raw)
To: jim_baxter, linux-i2c; +Cc: Peter Korsgaard, Wolfram Sang, linux-kernel
In-Reply-To: <1473278729-16329-1-git-send-email-jim_baxter@mentor.com>
On 2016-09-07 22:05, jim_baxter@mentor.com wrote:
> From: Jim Baxter <jim_baxter@mentor.com>
>
> This patchset adds a new i2c_mux_add_reparented_adapter API to the i2c
> that allows owning modules to use module_get/module_put and stop the
> i2c bus module being removed whilst in use.
>
> This was tested on an ARM i.MX6 Sabre board with the pca953x gpio module.
>
> Joshua Frkuska (2):
> i2c-mux: add i2c_mux_add_reparented_adapter api
> i2c-mux-gpio: call i2c_add_reparented_mux_adapter
nitpick: Patch subjects for the second patch is wrong.
"reparented" is a bit dual when dealing with i2c adapter trees.
i2c_mux_add_owned_adapter is perhaps clearer?
Aside from that, I'm not using modules much and need some enlightenment
as to why the i2c_del_mux_adapter() call in i2c_mux_gpio_remove() is not
sufficient and what exactly the problem is? Why would someone/something
unload the i2c-mux module prematurely?
Would it be an alternative to make i2c-mux a proper kernel object of
some kind? I mean, why do not all other mux users also need to modify
the owner? Why is i2c-mux-gpio special?
CHeers,
Peter
^ permalink raw reply
* Re: [PATCH v5] i2c: imx: make bus recovery through pinctrl optional
From: Stefan Agner @ 2016-09-09 20:34 UTC (permalink / raw)
To: Leo Li
Cc: Uwe Kleine-König, Gao Pan, Wolfram Sang, lkml, Li Yang,
linux-gpio, linux-i2c, linux-arm-kernel
In-Reply-To: <CADRPPNS3PecYRaAMR+0RuzU+gE3y5ZN-x7HzqggrdDsN-CkArg@mail.gmail.com>
On 2016-09-09 12:37, Leo Li wrote:
> On Fri, Sep 9, 2016 at 11:51 AM, Stefan Agner <stefan@agner.ch> wrote:
>> On 2016-09-08 16:57, Leo Li wrote:
>>> On Thu, Sep 8, 2016 at 5:39 PM, Stefan Agner <stefan@agner.ch> wrote:
>>>> On 2016-09-06 15:40, Leo Li wrote:
>>>>> On Tue, Sep 6, 2016 at 4:51 PM, Stefan Agner <stefan@agner.ch> wrote:
>>>>>> On 2016-09-06 13:06, Leo Li wrote:
>>>>>>> On Tue, Sep 6, 2016 at 1:58 PM, Uwe Kleine-König
>>>>>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>>>>>> On Fri, Aug 19, 2016 at 05:05:22PM -0500, Li Yang wrote:
>>>> <snip>
>>>>>>>>> @@ -1081,8 +1090,11 @@ static int i2c_imx_probe(struct platform_device *pdev)
>>>>>>>>> return ret;
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> + /* optional bus recovery feature through pinctrl */
>>>>>>>>> i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
>>>>>>>>> - if (IS_ERR(i2c_imx->pinctrl)) {
>>>>>>>>> + /* bailout on -ENOMEM or -EPROBE_DEFER, continue for other errors */
>>>>>>>>> + if (PTR_ERR(i2c_imx->pinctrl) == -ENOMEM ||
>>>>>>>>> + PTR_ERR(i2c_imx->pinctrl) == -EPROBE_DEFER) {
>>>>>>>>> ret = PTR_ERR(i2c_imx->pinctrl);
>>>>>>>>> goto clk_disable;
>>>>>>>>> }
>>>>>>>>
>>>>>>>> devm_pinctrl_get might return the following error-valued pointers:
>>>>>>>> - -EINVAL
>>>>>>>> - -ENOMEM
>>>>>>>> - -ENODEV
>>>>>>>> - -EPROBE_DEFER
>>>>>>>>
>>>>>>>> There are several error paths returning -EINVAL, one is when an invalid
>>>>>>>> phandle is used. Do you really want to ignore that?
>>>>>>>>
>>>>>>>> IMO error handling is better done with inverse logic, that is continue
>>>>>>>> on some explicit error, bail out on all unknown stuff. This tends to be
>>>>>>>> more robust. Also the comment should be improved to not explain that for
>>>>>>>> -ENOMEM and -EPROBE_DEFER we bail out (which should be obvious for
>>>>>>>> anyone who can read C) but to explain why.
>>>>>>>
>>>>>>> What you said is true for normal error handling, but in this scenario
>>>>>>> it is intentional to ignore all pinctrl related errors except critical
>>>>>>> ones because failing to have pinctrl for an optional feature shouldn't
>>>>>>> impact the function of normal i2c. We choose to catch -ENOMEM because
>>>>>>> the error could also cause problem for i2c probe, and -EPROBE_DEFER
>>>>>>> because it's possible that the pinctrl will be ready later and we want
>>>>>>> to give it a chance. The i2c driver really don't care why the pinctrl
>>>>>>> was not usable. I thought I added comment before the
>>>>>>
>>>>>> I don't agree. E.g. -EINVAL would appear if you pass devm_pinctrl_get an
>>>>>> invalid device. Currently you would silently ignore that, which is not
>>>>>> what you want.
>>>>>
>>>>> It is not silently ignored, there will be a message printed out saying
>>>>> pinctrl is not available and bus recovery is not supported. On the
>>>>> contrary, without this change the entire i2c driver fails to work
>>>>> silently if pinctrl is somehow not working. And if the system is so
>>>>> broken that the pointer to the i2c device is NULL, the probe of i2c
>>>>> would have already failed before this point. We shouldn't count on an
>>>>> optional function of the driver to catch fundamental issues like this.
>>>>>
>>>>>>
>>>>>> You want to get the pinctrl in any case expect there isn't one. And that
>>>>>> is how you should formulate your if statement.
>>>>>>
>>>>>> /*
>>>>>> * It is ok if no pinctrl device is available. We'll not be able to use
>>>>>> the
>>>>>> * bus recovery feature, but otherwise the driver works fine...
>>>>>> */
>>>>>> if (PTR_ERR(i2c_imx->pinctrl) != -ENODEV)
>>>>>
>>>>> I agree that there could be other possibilities that the pinctrl
>>>>> failed to work beside the reason I described in the commit
>>>>> message(platform doesn't support pinctrl at all). But I don't think
>>>>> any of them other than the -ENOMEM and -EPROBE_DEFER deserves a bail
>>>>> out for the entire i2c driver.
>>>>
>>>> FWIW, I disagree. If there is pinctrl defined, you want be sure that it
>>>> gets applied properly, no matter what. E.g. when devm_pinctrl_get return
>>>> EINVAL (Uwe's example) the driver will continue and likely fail in
>>>> mysterious ways later on because the pins have not been muxed properly.
>>>> The driver should not load in that situation so that the developer is
>>>> forced to fix his mistakes. The only reason to bail out here is if there
>>>> is no pin controller (ENODEV). And it seems that Uwe also tends to that
>>>> solution.
>>>
>>> With this patch the i2c bus recovery feature will be disabled if the
>>> devm_pinctrl_get() fails. The pin mux setting will not be changed in
>>> either i2c probe stage or at runtime. I don't think it can cause any
>>> trouble to normal I2C operation. IMO, it is not good to *force*
>>
>> If you have a pin controller, and you make a typo in your device tree
>> which leads to a wrong phandle and devm_pinctrl_get returning -EINVAL,
>> the system won't mux the pins... And that will certainly affect normal
>> I2C operation!
>
> I see why we are having different understanding now. The i2c-imx
> driver doesn't rely on the pinctrl to get the correct pin-mux setting
> for I2C operation. The pinctrl code was just added for recently for
> the bus recovery function. The assumption is that the correct
> pin-muxing has been taken care of by platform code or reset logic.
> The pinctrl is only used to enter/exit bus recovery mode, nothing
> else. Hence optional. So a typo in the pinctrl node only impacts bus
> recovery function.
>
You are right, the framework usually makes sure that default pinctrl
gets applied. I got that wrong.
>>
>>> people fix problem that they don't really care by deliberately enlarge
>>> the problem. That's why we don't panic() on any error we found. For
>>> those who do care about the bus recovery, they can get the information
>>> from the console.
>>
>> IMHO, it is just stupid to ignore errors and then let the developer
>> later on trace back what the initial issue was. Error out early is a
>> common sense software design principle...
>>
>> I am not asking for a panic(), I am just suggesting to only ignore
>> pinctrl if it returns -ENODEV, the case you care are about.
>
> It was just an analogy for enlarging the problem for getting
> attention. But you probably thought that it was not enlarging the
> problem as you think pinctrl is required by the driver instead of an
> optional thing.
Yeah it is a bit a wording thing: In my understanding, pinctrl is
required on SoC's witch have a pin controller... It is just that the
driver does not need to get the pinctrl by itself because the stack is
taking care of it implicitly. And yes, that makes the particular example
not a real world example.
However, I still feel that only bailing out if -ENODEV is returned is
the cleaner solution, and expresses better what your intention is. But
then I am not the pinctrl maintainer and I don't feel that it is worth
debating more around it, so whatever...
--
Stefan
^ permalink raw reply
* [PATCH 1/2] i2c: aspeed: added driver for Aspeed I2C
From: Brendan Higgins @ 2016-09-09 19:54 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland
Cc: linux-i2c, devicetree, linux-kernel, openbmc, joel, jk,
Brendan Higgins
In-Reply-To: <1473450870-10333-1-git-send-email-brendanhiggins@google.com>
Added initial master and slave support for Aspeed I2C controller.
Supports fourteen busses present in ast24xx and ast25xx BMC SoCs by
Aspeed.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-aspeed.c | 816 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 827 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-aspeed.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 5c3993b..0178c6c 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -998,6 +998,16 @@ config I2C_RCAR
This driver can also be built as a module. If so, the module
will be called i2c-rcar.
+config I2C_ASPEED
+ tristate "Aspeed AST2xxx SoC I2C Controller"
+ depends on (ARCH_ASPEED || COMPILE_TEST) && OF
+ help
+ If you say yes to this option, support will be included for the
+ Aspeed AST2xxx SoC I2C controller.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-aspeed.
+
comment "External I2C/SMBus adapter drivers"
config I2C_DIOLAN_U2C
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 37f2819..49631cd 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -96,6 +96,7 @@ obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o
obj-$(CONFIG_I2C_XLR) += i2c-xlr.o
obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o
obj-$(CONFIG_I2C_RCAR) += i2c-rcar.o
+obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
# External I2C/SMBus adapter drivers
obj-$(CONFIG_I2C_DIOLAN_U2C) += i2c-diolan-u2c.o
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
new file mode 100644
index 0000000..9a5ca04
--- /dev/null
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -0,0 +1,816 @@
+/*
+ * I2C adapter for the ASPEED I2C bus.
+ *
+ * Copyright (C) 2012-2020 ASPEED Technology Inc.
+ * Copyright 2016 IBM Corporation
+ * Copyright 2016 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/completion.h>
+#include <linux/slab.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+
+/* I2C Register */
+#define ASPEED_I2C_FUN_CTRL_REG 0x00
+#define ASPEED_I2C_AC_TIMING_REG1 0x04
+#define ASPEED_I2C_AC_TIMING_REG2 0x08
+#define ASPEED_I2C_INTR_CTRL_REG 0x0c
+#define ASPEED_I2C_INTR_STS_REG 0x10
+#define ASPEED_I2C_CMD_REG 0x14
+#define ASPEED_I2C_DEV_ADDR_REG 0x18
+#define ASPEED_I2C_BYTE_BUF_REG 0x20
+
+#define ASPEED_I2C_NUM_BUS 14
+
+/* Global Register Definition */
+/* 0x00 : I2C Interrupt Status Register */
+/* 0x08 : I2C Interrupt Target Assignment */
+
+/* Device Register Definition */
+/* 0x00 : I2CD Function Control Register */
+#define ASPEED_I2CD_MULTI_MASTER_DIS BIT(15)
+#define ASPEED_I2CD_SDA_DRIVE_1T_EN BIT(8)
+#define ASPEED_I2CD_M_SDA_DRIVE_1T_EN BIT(7)
+#define ASPEED_I2CD_M_HIGH_SPEED_EN BIT(6)
+#define ASPEED_I2CD_SLAVE_EN BIT(1)
+#define ASPEED_I2CD_MASTER_EN BIT(0)
+
+/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
+#define ASPEED_NO_TIMEOUT_CTRL 0
+
+
+/* 0x0c : I2CD Interrupt Control Register &
+ * 0x10 : I2CD Interrupt Status Register
+ *
+ * These share bit definitions, so use the same values for the enable &
+ * status bits.
+ */
+#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT BIT(14)
+#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE BIT(13)
+#define ASPEED_I2CD_INTR_SLAVE_MATCH BIT(7)
+#define ASPEED_I2CD_INTR_SCL_TIMEOUT BIT(6)
+#define ASPEED_I2CD_INTR_ABNORMAL BIT(5)
+#define ASPEED_I2CD_INTR_NORMAL_STOP BIT(4)
+#define ASPEED_I2CD_INTR_ARBIT_LOSS BIT(3)
+#define ASPEED_I2CD_INTR_RX_DONE BIT(2)
+#define ASPEED_I2CD_INTR_TX_NAK BIT(1)
+#define ASPEED_I2CD_INTR_TX_ACK BIT(0)
+
+/* 0x14 : I2CD Command/Status Register */
+#define ASPEED_I2CD_SCL_LINE_STS BIT(18)
+#define ASPEED_I2CD_SDA_LINE_STS BIT(17)
+#define ASPEED_I2CD_BUS_BUSY_STS BIT(16)
+#define ASPEED_I2CD_BUS_RECOVER_CMD BIT(11)
+
+/* Command Bit */
+#define ASPEED_I2CD_M_STOP_CMD BIT(5)
+#define ASPEED_I2CD_M_S_RX_CMD_LAST BIT(4)
+#define ASPEED_I2CD_M_RX_CMD BIT(3)
+#define ASPEED_I2CD_S_TX_CMD BIT(2)
+#define ASPEED_I2CD_M_TX_CMD BIT(1)
+#define ASPEED_I2CD_M_START_CMD BIT(0)
+
+/* 0x18 : I2CD Slave Device Address Register */
+#define ASPEED_I2CD_DEV_ADDR_MASK GENMASK(6, 0)
+
+enum aspeed_i2c_slave_state {
+ ASPEED_I2C_SLAVE_START,
+ ASPEED_I2C_SLAVE_READ_REQUESTED,
+ ASPEED_I2C_SLAVE_READ_PROCESSED,
+ ASPEED_I2C_SLAVE_WRITE_REQUESTED,
+ ASPEED_I2C_SLAVE_WRITE_RECEIVED,
+ ASPEED_I2C_SLAVE_STOP,
+};
+
+struct aspeed_i2c_bus {
+ struct i2c_adapter adap;
+ struct device *dev;
+ void __iomem *base;
+ spinlock_t lock;
+ struct completion cmd_complete;
+ int irq;
+ /* Transaction state. */
+ struct i2c_msg *msg;
+ int msg_pos;
+ u32 cmd_err;
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ struct i2c_client *slave;
+ enum aspeed_i2c_slave_state slave_state;
+#endif
+};
+
+struct aspeed_i2c_controller {
+ struct device *dev;
+ void __iomem *base;
+ int irq;
+ struct irq_domain *irq_domain;
+};
+
+static inline void aspeed_i2c_write(struct aspeed_i2c_bus *bus, u32 val,
+ u32 reg)
+{
+ writel(val, bus->base + reg);
+}
+
+static inline u32 aspeed_i2c_read(struct aspeed_i2c_bus *bus, u32 reg)
+{
+ return readl(bus->base + reg);
+}
+
+static u8 aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
+{
+ u32 command;
+ unsigned long time_left;
+ unsigned long flags;
+ int ret = 0;
+
+ spin_lock_irqsave(&bus->lock, flags);
+ command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
+ /* Bus is idle: no recovery needed. */
+ if ((command & ASPEED_I2CD_SDA_LINE_STS) &&
+ (command & ASPEED_I2CD_SCL_LINE_STS))
+ goto out;
+
+ dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
+ command);
+
+ /* Bus held: put bus in stop state. */
+ if ((command & ASPEED_I2CD_SDA_LINE_STS) &&
+ !(command & ASPEED_I2CD_SCL_LINE_STS)) {
+ aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
+ ASPEED_I2C_CMD_REG);
+ reinit_completion(&bus->cmd_complete);
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ time_left = wait_for_completion_interruptible_timeout(
+ &bus->cmd_complete, bus->adap.timeout * HZ);
+
+ spin_lock_irqsave(&bus->lock, flags);
+ if (time_left == 0)
+ ret = -ETIMEDOUT;
+ else if (bus->cmd_err)
+ ret = -EIO;
+ /* Bus error. */
+ } else if (!(command & ASPEED_I2CD_SDA_LINE_STS)) {
+ aspeed_i2c_write(bus, ASPEED_I2CD_BUS_RECOVER_CMD,
+ ASPEED_I2C_CMD_REG);
+ reinit_completion(&bus->cmd_complete);
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ time_left = wait_for_completion_interruptible_timeout(
+ &bus->cmd_complete, bus->adap.timeout * HZ);
+
+ spin_lock_irqsave(&bus->lock, flags);
+ if (time_left == 0)
+ ret = -ETIMEDOUT;
+ else if (bus->cmd_err)
+ ret = -EIO;
+ /* Recovery failed. */
+ else if (!(aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
+ ASPEED_I2CD_SDA_LINE_STS))
+ ret = -EIO;
+ }
+
+out:
+ spin_unlock_irqrestore(&bus->lock, flags);
+ return ret;
+}
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
+{
+ bool irq_handled = true;
+ u32 command;
+ u32 irq_status;
+ u32 status_ack = 0;
+ u8 value;
+ struct i2c_client *slave = bus->slave;
+
+ spin_lock(&bus->lock);
+ if (!slave) {
+ irq_handled = false;
+ goto out;
+ }
+ command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
+ irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
+
+ /* Slave was requested, restart state machine. */
+ if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
+ status_ack |= ASPEED_I2CD_INTR_SLAVE_MATCH;
+ bus->slave_state = ASPEED_I2C_SLAVE_START;
+ }
+ /* Slave is not currently active, irq was for someone else. */
+ if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
+ irq_handled = false;
+ goto out;
+ }
+
+ dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
+ irq_status, command);
+
+ /* Slave was sent something. */
+ if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
+ value = aspeed_i2c_read(bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
+ /* Handle address frame. */
+ if (bus->slave_state == ASPEED_I2C_SLAVE_START) {
+ if (value & 0x1)
+ bus->slave_state =
+ ASPEED_I2C_SLAVE_READ_REQUESTED;
+ else
+ bus->slave_state =
+ ASPEED_I2C_SLAVE_WRITE_REQUESTED;
+ }
+ status_ack |= ASPEED_I2CD_INTR_RX_DONE;
+ }
+
+ /* Slave was asked to stop. */
+ if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
+ status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
+ bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+ }
+ if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
+ status_ack |= ASPEED_I2CD_INTR_TX_NAK;
+ bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+ }
+
+ if (bus->slave_state == ASPEED_I2C_SLAVE_READ_REQUESTED) {
+ if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
+ dev_err(bus->dev, "Unexpected ACK on read request.\n");
+ bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED;
+
+ i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
+ aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
+ aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
+ } else if (bus->slave_state == ASPEED_I2C_SLAVE_READ_PROCESSED) {
+ status_ack |= ASPEED_I2CD_INTR_TX_ACK;
+ if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
+ dev_err(bus->dev,
+ "Expected ACK after processed read.\n");
+ i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
+ aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
+ aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
+ } else if (bus->slave_state == ASPEED_I2C_SLAVE_WRITE_REQUESTED) {
+ bus->slave_state = ASPEED_I2C_SLAVE_WRITE_RECEIVED;
+ i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
+ } else if (bus->slave_state == ASPEED_I2C_SLAVE_WRITE_RECEIVED) {
+ i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
+ } else if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
+ i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
+ }
+
+ if (status_ack != irq_status)
+ dev_err(bus->dev,
+ "irq handled != irq. expected %x, but was %x\n",
+ irq_status, status_ack);
+ aspeed_i2c_write(bus, status_ack, ASPEED_I2C_INTR_STS_REG);
+
+out:
+ spin_unlock(&bus->lock);
+ return irq_handled;
+}
+#endif
+
+static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
+{
+ const u32 errs = ASPEED_I2CD_INTR_ARBIT_LOSS |
+ ASPEED_I2CD_INTR_ABNORMAL |
+ ASPEED_I2CD_INTR_SCL_TIMEOUT |
+ ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |
+ ASPEED_I2CD_INTR_TX_NAK;
+ u32 irq_status;
+
+ spin_lock(&bus->lock);
+ irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
+ bus->cmd_err = irq_status & errs;
+
+ dev_dbg(bus->dev, "master irq status 0x%08x\n", irq_status);
+
+ /* No message to transfer. */
+ if (bus->cmd_err ||
+ (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) ||
+ (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE)) {
+ complete(&bus->cmd_complete);
+ goto out;
+ } else if (!bus->msg || bus->msg_pos >= bus->msg->len)
+ goto out;
+
+ if ((bus->msg->flags & I2C_M_RD) &&
+ (irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
+ bus->msg->buf[bus->msg_pos++] = aspeed_i2c_read(
+ bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
+ if (bus->msg_pos + 1 < bus->msg->len)
+ aspeed_i2c_write(bus, ASPEED_I2CD_M_RX_CMD,
+ ASPEED_I2C_CMD_REG);
+ else if (bus->msg_pos < bus->msg->len)
+ aspeed_i2c_write(bus, ASPEED_I2CD_M_RX_CMD |
+ ASPEED_I2CD_M_S_RX_CMD_LAST,
+ ASPEED_I2C_CMD_REG);
+ } else if (!(bus->msg->flags & I2C_M_RD) &&
+ (irq_status & ASPEED_I2CD_INTR_TX_ACK)) {
+ aspeed_i2c_write(bus, bus->msg->buf[bus->msg_pos++],
+ ASPEED_I2C_BYTE_BUF_REG);
+ aspeed_i2c_write(bus, ASPEED_I2CD_M_TX_CMD, ASPEED_I2C_CMD_REG);
+ }
+
+ /* Transmission complete: notify caller. */
+ if (bus->msg_pos >= bus->msg->len)
+ complete(&bus->cmd_complete);
+out:
+ aspeed_i2c_write(bus, irq_status, ASPEED_I2C_INTR_STS_REG);
+ spin_unlock(&bus->lock);
+ return true;
+}
+
+static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
+{
+ struct aspeed_i2c_bus *bus = dev_id;
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ if (aspeed_i2c_slave_irq(bus)) {
+ dev_dbg(bus->dev, "irq handled by slave.\n");
+ return IRQ_HANDLED;
+ }
+#endif
+ if (aspeed_i2c_master_irq(bus)) {
+ dev_dbg(bus->dev, "irq handled by master.\n");
+ return IRQ_HANDLED;
+ }
+ dev_err(bus->dev, "irq not handled properly!\n");
+ return IRQ_HANDLED;
+}
+
+static int aspeed_i2c_master_single_xfer(struct i2c_adapter *adap,
+ struct i2c_msg *msg)
+{
+ struct aspeed_i2c_bus *bus = adap->algo_data;
+ unsigned long flags;
+ u8 slave_addr;
+ u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
+ int ret = msg->len;
+ unsigned long time_left;
+
+ spin_lock_irqsave(&bus->lock, flags);
+ bus->msg = msg;
+ bus->msg_pos = 0;
+ slave_addr = msg->addr << 1;
+ if (msg->flags & I2C_M_RD) {
+ slave_addr |= 1;
+ command |= ASPEED_I2CD_M_RX_CMD;
+ if (msg->len == 1)
+ command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+ }
+ aspeed_i2c_write(bus, slave_addr, ASPEED_I2C_BYTE_BUF_REG);
+ aspeed_i2c_write(bus, command, ASPEED_I2C_CMD_REG);
+ reinit_completion(&bus->cmd_complete);
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ time_left = wait_for_completion_interruptible_timeout(
+ &bus->cmd_complete, bus->adap.timeout * HZ * msg->len);
+ if (time_left == 0)
+ return -ETIMEDOUT;
+
+ spin_lock_irqsave(&bus->lock, flags);
+ if (bus->cmd_err)
+ ret = -EIO;
+ bus->msg = NULL;
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ return ret;
+}
+
+static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
+ struct i2c_msg *msgs, int num)
+{
+ struct aspeed_i2c_bus *bus = adap->algo_data;
+ int ret;
+ int i;
+ unsigned long flags;
+ unsigned long time_left;
+
+ /* If bus is busy, attempt recovery. We assume a single master
+ * environment.
+ */
+ if (aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
+ ASPEED_I2CD_BUS_BUSY_STS) {
+ ret = aspeed_i2c_recover_bus(bus);
+ if (ret)
+ return ret;
+ }
+
+ for (i = 0; i < num; i++) {
+ ret = aspeed_i2c_master_single_xfer(adap, &msgs[i]);
+ if (ret < 0)
+ break;
+ /* TODO: Support other forms of I2C protocol mangling. */
+ if (msgs[i].flags & I2C_M_STOP) {
+ spin_lock_irqsave(&bus->lock, flags);
+ aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
+ ASPEED_I2C_CMD_REG);
+ reinit_completion(&bus->cmd_complete);
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ time_left = wait_for_completion_interruptible_timeout(
+ &bus->cmd_complete,
+ bus->adap.timeout * HZ);
+ if (time_left == 0)
+ return -ETIMEDOUT;
+ }
+ }
+
+ spin_lock_irqsave(&bus->lock, flags);
+ aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD, ASPEED_I2C_CMD_REG);
+ reinit_completion(&bus->cmd_complete);
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ time_left = wait_for_completion_interruptible_timeout(
+ &bus->cmd_complete, bus->adap.timeout * HZ);
+ if (time_left == 0)
+ return -ETIMEDOUT;
+
+ /* If nothing went wrong, return number of messages transferred. */
+ if (ret < 0)
+ return ret;
+ else
+ return i;
+}
+
+static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static int aspeed_i2c_reg_slave(struct i2c_client *client)
+{
+ struct aspeed_i2c_bus *bus;
+ unsigned long flags;
+ u32 addr_reg_val;
+ u32 func_ctrl_reg_val;
+
+ bus = client->adapter->algo_data;
+ spin_lock_irqsave(&bus->lock, flags);
+ if (bus->slave) {
+ spin_unlock_irqrestore(&bus->lock, flags);
+ return -EINVAL;
+ }
+
+ /* Set slave addr. */
+ addr_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_DEV_ADDR_REG);
+ addr_reg_val &= ~ASPEED_I2CD_DEV_ADDR_MASK;
+ addr_reg_val |= client->addr & ASPEED_I2CD_DEV_ADDR_MASK;
+ aspeed_i2c_write(bus, addr_reg_val, ASPEED_I2C_DEV_ADDR_REG);
+
+ /* Switch from master mode to slave mode. */
+ func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
+ func_ctrl_reg_val &= ~ASPEED_I2CD_MASTER_EN;
+ func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
+ aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
+
+ bus->slave = client;
+ bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+ spin_unlock_irqrestore(&bus->lock, flags);
+ return 0;
+}
+
+static int aspeed_i2c_unreg_slave(struct i2c_client *client)
+{
+ struct aspeed_i2c_bus *bus = client->adapter->algo_data;
+ unsigned long flags;
+ u32 func_ctrl_reg_val;
+
+ spin_lock_irqsave(&bus->lock, flags);
+ if (!bus->slave) {
+ spin_unlock_irqrestore(&bus->lock, flags);
+ return -EINVAL;
+ }
+
+ /* Switch from slave mode to master mode. */
+ func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
+ func_ctrl_reg_val &= ~ASPEED_I2CD_SLAVE_EN;
+ func_ctrl_reg_val |= ASPEED_I2CD_MASTER_EN;
+ aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
+
+ bus->slave = NULL;
+ spin_unlock_irqrestore(&bus->lock, flags);
+ return 0;
+}
+#endif
+
+static const struct i2c_algorithm aspeed_i2c_algo = {
+ .master_xfer = aspeed_i2c_master_xfer,
+ .functionality = aspeed_i2c_functionality,
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ .reg_slave = aspeed_i2c_reg_slave,
+ .unreg_slave = aspeed_i2c_unreg_slave,
+#endif
+};
+
+static u32 aspeed_i2c_get_clk_reg_val(u32 divider_ratio)
+{
+ unsigned int inc = 0, div;
+ u32 scl_low, scl_high, data;
+
+ for (div = 0; divider_ratio >= 16; div++) {
+ inc |= (divider_ratio & 1);
+ divider_ratio >>= 1;
+ }
+ divider_ratio += inc;
+ scl_low = (divider_ratio >> 1) - 1;
+ scl_high = divider_ratio - scl_low - 2;
+ data = 0x77700300 | (scl_high << 16) | (scl_low << 12) | div;
+ return data;
+}
+
+static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
+ struct platform_device *pdev)
+{
+ struct clk *pclk;
+ u32 clk_freq;
+ u32 divider_ratio;
+ int ret;
+
+ pclk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(pclk)) {
+ dev_err(&pdev->dev, "clk_get failed\n");
+ return PTR_ERR(pclk);
+ }
+ ret = of_property_read_u32(pdev->dev.of_node,
+ "clock-frequency", &clk_freq);
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "Could not read clock-frequency property\n");
+ clk_freq = 100000;
+ }
+ divider_ratio = clk_get_rate(pclk) / clk_freq;
+ /* We just need the clock rate, we don't actually use the clk object. */
+ devm_clk_put(&pdev->dev, pclk);
+
+ /* Set AC Timing */
+ if (clk_freq / 1000 > 400) {
+ aspeed_i2c_write(bus, aspeed_i2c_read(bus,
+ ASPEED_I2C_FUN_CTRL_REG) |
+ ASPEED_I2CD_M_HIGH_SPEED_EN |
+ ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
+ ASPEED_I2CD_SDA_DRIVE_1T_EN,
+ ASPEED_I2C_FUN_CTRL_REG);
+
+ aspeed_i2c_write(bus, 0x3, ASPEED_I2C_AC_TIMING_REG2);
+ aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divider_ratio),
+ ASPEED_I2C_AC_TIMING_REG1);
+ } else {
+ aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divider_ratio),
+ ASPEED_I2C_AC_TIMING_REG1);
+ aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
+ ASPEED_I2C_AC_TIMING_REG2);
+ }
+
+ return 0;
+}
+
+static void noop(struct irq_data *data) { }
+
+static struct irq_chip aspeed_i2c_irqchip = {
+ .name = "ast-i2c",
+ .irq_unmask = noop,
+ .irq_mask = noop,
+};
+
+static int aspeed_i2c_probe_bus(struct platform_device *pdev)
+{
+ struct aspeed_i2c_bus *bus;
+ struct aspeed_i2c_controller *controller =
+ dev_get_drvdata(pdev->dev.parent);
+ struct resource *res;
+ int ret, bus_num, irq;
+
+ bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
+ if (!bus)
+ return -ENOMEM;
+
+ ret = of_property_read_u32(pdev->dev.of_node, "bus", &bus_num);
+ if (ret)
+ return -ENXIO;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ bus->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(bus->base))
+ return PTR_ERR(bus->base);
+
+ bus->irq = platform_get_irq(pdev, 0);
+ if (bus->irq < 0) {
+ dev_err(&pdev->dev, "platform_get_irq failed\n");
+ return -ENXIO;
+ }
+
+ irq = irq_create_mapping(controller->irq_domain, bus_num);
+ irq_set_chip_data(irq, controller);
+ irq_set_chip_and_handler(irq, &aspeed_i2c_irqchip, handle_simple_irq);
+ ret = devm_request_irq(&pdev->dev, bus->irq, aspeed_i2c_bus_irq,
+ 0, dev_name(&pdev->dev), bus);
+ if (ret) {
+ dev_err(&pdev->dev, "devm_request_irq failed\n");
+ return -ENXIO;
+ }
+
+ /* Initialize the I2C adapter */
+ spin_lock_init(&bus->lock);
+ init_completion(&bus->cmd_complete);
+ bus->adap.nr = bus_num;
+ bus->adap.owner = THIS_MODULE;
+ bus->adap.retries = 0;
+ bus->adap.timeout = 5;
+ bus->adap.algo = &aspeed_i2c_algo;
+ bus->adap.algo_data = bus;
+ bus->adap.dev.parent = &pdev->dev;
+ bus->adap.dev.of_node = pdev->dev.of_node;
+ snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed i2c-%d",
+ bus_num);
+
+ bus->dev = &pdev->dev;
+
+ /* reset device: disable master & slave functions */
+ aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
+
+ ret = aspeed_i2c_init_clk(bus, pdev);
+ if (ret < 0)
+ return ret;
+
+ /* Enable Master Mode */
+ aspeed_i2c_write(bus, aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG) |
+ ASPEED_I2CD_MASTER_EN |
+ ASPEED_I2CD_MULTI_MASTER_DIS, ASPEED_I2C_FUN_CTRL_REG);
+
+ /* Set interrupt generation of I2C controller */
+ aspeed_i2c_write(bus, ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |
+ ASPEED_I2CD_INTR_BUS_RECOVER_DONE |
+ ASPEED_I2CD_INTR_SCL_TIMEOUT |
+ ASPEED_I2CD_INTR_ABNORMAL |
+ ASPEED_I2CD_INTR_NORMAL_STOP |
+ ASPEED_I2CD_INTR_ARBIT_LOSS |
+ ASPEED_I2CD_INTR_RX_DONE |
+ ASPEED_I2CD_INTR_TX_NAK |
+ ASPEED_I2CD_INTR_TX_ACK,
+ ASPEED_I2C_INTR_CTRL_REG);
+
+ ret = i2c_add_numbered_adapter(&bus->adap);
+ if (ret < 0)
+ return -ENXIO;
+
+ platform_set_drvdata(pdev, bus);
+
+ dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
+ bus->adap.nr, bus->irq);
+
+ return 0;
+}
+
+static int aspeed_i2c_remove_bus(struct platform_device *pdev)
+{
+ struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
+
+ i2c_del_adapter(&bus->adap);
+ return 0;
+}
+
+static const struct of_device_id aspeed_i2c_bus_of_table[] = {
+ { .compatible = "aspeed,ast2400-i2c-bus", },
+ { .compatible = "aspeed,ast2500-i2c-bus", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, aspeed_i2c_of_table);
+
+static struct platform_driver aspeed_i2c_bus_driver = {
+ .probe = aspeed_i2c_probe_bus,
+ .remove = aspeed_i2c_remove_bus,
+ .driver = {
+ .name = "ast-i2c-bus",
+ .of_match_table = aspeed_i2c_bus_of_table,
+ },
+};
+module_platform_driver(aspeed_i2c_bus_driver);
+
+static void aspeed_i2c_controller_irq(struct irq_desc *desc)
+{
+ struct aspeed_i2c_controller *c = irq_desc_get_handler_data(desc);
+ unsigned long p, status;
+ unsigned int bus_irq;
+
+ status = readl(c->base);
+ for_each_set_bit(p, &status, ASPEED_I2C_NUM_BUS) {
+ bus_irq = irq_find_mapping(c->irq_domain, p);
+ generic_handle_irq(bus_irq);
+ }
+}
+
+static int aspeed_i2c_probe_controller(struct platform_device *pdev)
+{
+ struct aspeed_i2c_controller *controller;
+ struct device_node *np;
+ struct resource *res;
+
+ controller = kzalloc(sizeof(*controller), GFP_KERNEL);
+ if (!controller)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ controller->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(controller->base))
+ return PTR_ERR(controller->base);
+
+ controller->irq = platform_get_irq(pdev, 0);
+ if (controller->irq < 0) {
+ dev_err(&pdev->dev, "no platform IRQ\n");
+ return -ENXIO;
+ }
+
+ controller->irq_domain = irq_domain_add_linear(pdev->dev.of_node,
+ ASPEED_I2C_NUM_BUS, &irq_domain_simple_ops, NULL);
+ if (!controller->irq_domain) {
+ dev_err(&pdev->dev, "no IRQ domain\n");
+ return -ENXIO;
+ }
+ controller->irq_domain->name = "ast-i2c-domain";
+
+ irq_set_chained_handler_and_data(controller->irq,
+ aspeed_i2c_controller_irq, controller);
+
+ controller->dev = &pdev->dev;
+
+ platform_set_drvdata(pdev, controller);
+
+ dev_info(controller->dev, "i2c controller registered, irq %d\n",
+ controller->irq);
+
+ for_each_child_of_node(pdev->dev.of_node, np) {
+ int ret;
+ u32 bus_num;
+ char bus_id[sizeof("i2c-12345")];
+
+ /*
+ * Set a useful name derived from the bus number; the device
+ * tree should provide us with one that corresponds to the
+ * hardware numbering. If the property is missing the
+ * probe would fail so just skip it here.
+ */
+
+ ret = of_property_read_u32(np, "bus", &bus_num);
+ if (ret)
+ continue;
+
+ ret = snprintf(bus_id, sizeof(bus_id), "i2c-%u", bus_num);
+ if (ret >= sizeof(bus_id))
+ continue;
+
+ of_platform_device_create(np, bus_id, &pdev->dev);
+ of_node_put(np);
+ }
+
+ return 0;
+}
+
+static int aspeed_i2c_remove_controller(struct platform_device *pdev)
+{
+ struct aspeed_i2c_controller *controller = platform_get_drvdata(pdev);
+
+ irq_domain_remove(controller->irq_domain);
+ return 0;
+}
+
+static const struct of_device_id aspeed_i2c_controller_of_table[] = {
+ { .compatible = "aspeed,ast2400-i2c-controller", },
+ { .compatible = "aspeed,ast2500-i2c-controller", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, aspeed_i2c_of_table);
+
+static struct platform_driver aspeed_i2c_controller_driver = {
+ .probe = aspeed_i2c_probe_controller,
+ .remove = aspeed_i2c_remove_controller,
+ .driver = {
+ .name = "ast-i2c-controller",
+ .of_match_table = aspeed_i2c_controller_of_table,
+ },
+};
+
+module_platform_driver(aspeed_i2c_controller_driver);
+
+MODULE_AUTHOR("Brendan Higgins <brendanhiggins@google.com>");
+MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
+MODULE_LICENSE("GPL");
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH 0/2] i2c: aspeed: added driver for Aspeed I2C
From: Brendan Higgins @ 2016-09-09 19:54 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland
Cc: linux-i2c, devicetree, linux-kernel, openbmc, joel, jk
This patch set adds initial master mode and slave mode support for the Aspeed
24xx and 25xx I2C controllers, as well as device binding documentation for the
new driver.
These changes have been tested on the ast2500 evaluation board from Aspeed.
Work still to be done:
- The driver supports I2C protocol mangling in the form of sending stops after
every transaction, the remaining mangling support has not yet been added.
- SMBus Alert support.
Cheers!
^ permalink raw reply
* [PATCH 2/2] i2c: aspeed: added documentation for Aspeed I2C driver
From: Brendan Higgins @ 2016-09-09 19:54 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, joel-U3u1mxZcP9KHXe+LvDLADg,
jk-mnsaURCQ41sdnm+yROfE0A, Brendan Higgins
In-Reply-To: <1473450870-10333-1-git-send-email-brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Added device tree binding documentation for Aspeed I2C controller and
busses.
Signed-off-by: Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
.../devicetree/bindings/i2c/i2c-aspeed.txt | 63 ++++++++++++++++++++++
1 file changed, 63 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
new file mode 100644
index 0000000..df68f2a
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
@@ -0,0 +1,63 @@
+Device tree configuration for the I2C controller and busses on the AST24XX
+and AST25XX SoCs.
+
+Controller:
+
+ Required Properties:
+ - #address-cells : should be 1
+ - #size-cells : should be 1
+ - #interrupt-cells : should be 1
+ - compatible : should be "aspeed,ast2400-i2c-controller"
+ or "aspeed,ast2500-i2c-controller"
+ - reg : address start and range of controller
+ - ranges : defines address offset and range for busses
+ - interrupts : interrupt number
+ - clocks : root clock of bus, should reference the APB
+ clock
+ - clock-ranges : specifies that child busses can inherit clocks
+ - interrupt-controller : denotes that the controller receives and fires
+ new interrupts for child busses
+
+Bus:
+
+ Required Properties:
+ - #address-cells : should be 1
+ - #size-cells : should be 0
+ - reg : address offset and range of bus
+ - compatible : should be "aspeed,ast2400-i2c-bus"
+ or "aspeed,ast2500-i2c-bus"
+ - bus : the bus's number
+ - interrupts : interrupt number
+
+ Optional Properties:
+ - clock-frequency : frequency of the bus clock in Hz
+ defaults to 100 kHz when not specified
+
+Example:
+
+i2c: i2c@1e78a000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ #interrupt-cells = <1>;
+
+ compatible = "aspeed,ast2400-i2c-controller";
+ reg = <0x1e78a000 0x40>;
+ ranges = <0 0x1e78a000 0x1000>;
+ interrupts = <12>;
+ clocks = <&clk_apb>;
+ clock-ranges;
+ interrupt-controller;
+
+ i2c0: i2c-bus@40 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x40 0x40>;
+ compatible = "aspeed,ast2400-i2c-bus";
+ bus = <0>;
+ clock-frequency = <100000>;
+ status = "disabled";
+ interrupts = <0>;
+ interrupt-parent = <&i2c>;
+ };
+};
+
--
2.8.0.rc3.226.g39d4020
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v5] i2c: imx: make bus recovery through pinctrl optional
From: Leo Li @ 2016-09-09 19:37 UTC (permalink / raw)
To: Stefan Agner
Cc: Uwe Kleine-König, Gao Pan, Wolfram Sang, lkml, Li Yang,
linux-gpio, linux-i2c@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <306f579a2f2362595a55c53e48657563@agner.ch>
On Fri, Sep 9, 2016 at 11:51 AM, Stefan Agner <stefan@agner.ch> wrote:
> On 2016-09-08 16:57, Leo Li wrote:
>> On Thu, Sep 8, 2016 at 5:39 PM, Stefan Agner <stefan@agner.ch> wrote:
>>> On 2016-09-06 15:40, Leo Li wrote:
>>>> On Tue, Sep 6, 2016 at 4:51 PM, Stefan Agner <stefan@agner.ch> wrote:
>>>>> On 2016-09-06 13:06, Leo Li wrote:
>>>>>> On Tue, Sep 6, 2016 at 1:58 PM, Uwe Kleine-König
>>>>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>>>>> On Fri, Aug 19, 2016 at 05:05:22PM -0500, Li Yang wrote:
>>> <snip>
>>>>>>>> @@ -1081,8 +1090,11 @@ static int i2c_imx_probe(struct platform_device *pdev)
>>>>>>>> return ret;
>>>>>>>> }
>>>>>>>>
>>>>>>>> + /* optional bus recovery feature through pinctrl */
>>>>>>>> i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
>>>>>>>> - if (IS_ERR(i2c_imx->pinctrl)) {
>>>>>>>> + /* bailout on -ENOMEM or -EPROBE_DEFER, continue for other errors */
>>>>>>>> + if (PTR_ERR(i2c_imx->pinctrl) == -ENOMEM ||
>>>>>>>> + PTR_ERR(i2c_imx->pinctrl) == -EPROBE_DEFER) {
>>>>>>>> ret = PTR_ERR(i2c_imx->pinctrl);
>>>>>>>> goto clk_disable;
>>>>>>>> }
>>>>>>>
>>>>>>> devm_pinctrl_get might return the following error-valued pointers:
>>>>>>> - -EINVAL
>>>>>>> - -ENOMEM
>>>>>>> - -ENODEV
>>>>>>> - -EPROBE_DEFER
>>>>>>>
>>>>>>> There are several error paths returning -EINVAL, one is when an invalid
>>>>>>> phandle is used. Do you really want to ignore that?
>>>>>>>
>>>>>>> IMO error handling is better done with inverse logic, that is continue
>>>>>>> on some explicit error, bail out on all unknown stuff. This tends to be
>>>>>>> more robust. Also the comment should be improved to not explain that for
>>>>>>> -ENOMEM and -EPROBE_DEFER we bail out (which should be obvious for
>>>>>>> anyone who can read C) but to explain why.
>>>>>>
>>>>>> What you said is true for normal error handling, but in this scenario
>>>>>> it is intentional to ignore all pinctrl related errors except critical
>>>>>> ones because failing to have pinctrl for an optional feature shouldn't
>>>>>> impact the function of normal i2c. We choose to catch -ENOMEM because
>>>>>> the error could also cause problem for i2c probe, and -EPROBE_DEFER
>>>>>> because it's possible that the pinctrl will be ready later and we want
>>>>>> to give it a chance. The i2c driver really don't care why the pinctrl
>>>>>> was not usable. I thought I added comment before the
>>>>>
>>>>> I don't agree. E.g. -EINVAL would appear if you pass devm_pinctrl_get an
>>>>> invalid device. Currently you would silently ignore that, which is not
>>>>> what you want.
>>>>
>>>> It is not silently ignored, there will be a message printed out saying
>>>> pinctrl is not available and bus recovery is not supported. On the
>>>> contrary, without this change the entire i2c driver fails to work
>>>> silently if pinctrl is somehow not working. And if the system is so
>>>> broken that the pointer to the i2c device is NULL, the probe of i2c
>>>> would have already failed before this point. We shouldn't count on an
>>>> optional function of the driver to catch fundamental issues like this.
>>>>
>>>>>
>>>>> You want to get the pinctrl in any case expect there isn't one. And that
>>>>> is how you should formulate your if statement.
>>>>>
>>>>> /*
>>>>> * It is ok if no pinctrl device is available. We'll not be able to use
>>>>> the
>>>>> * bus recovery feature, but otherwise the driver works fine...
>>>>> */
>>>>> if (PTR_ERR(i2c_imx->pinctrl) != -ENODEV)
>>>>
>>>> I agree that there could be other possibilities that the pinctrl
>>>> failed to work beside the reason I described in the commit
>>>> message(platform doesn't support pinctrl at all). But I don't think
>>>> any of them other than the -ENOMEM and -EPROBE_DEFER deserves a bail
>>>> out for the entire i2c driver.
>>>
>>> FWIW, I disagree. If there is pinctrl defined, you want be sure that it
>>> gets applied properly, no matter what. E.g. when devm_pinctrl_get return
>>> EINVAL (Uwe's example) the driver will continue and likely fail in
>>> mysterious ways later on because the pins have not been muxed properly.
>>> The driver should not load in that situation so that the developer is
>>> forced to fix his mistakes. The only reason to bail out here is if there
>>> is no pin controller (ENODEV). And it seems that Uwe also tends to that
>>> solution.
>>
>> With this patch the i2c bus recovery feature will be disabled if the
>> devm_pinctrl_get() fails. The pin mux setting will not be changed in
>> either i2c probe stage or at runtime. I don't think it can cause any
>> trouble to normal I2C operation. IMO, it is not good to *force*
>
> If you have a pin controller, and you make a typo in your device tree
> which leads to a wrong phandle and devm_pinctrl_get returning -EINVAL,
> the system won't mux the pins... And that will certainly affect normal
> I2C operation!
I see why we are having different understanding now. The i2c-imx
driver doesn't rely on the pinctrl to get the correct pin-mux setting
for I2C operation. The pinctrl code was just added for recently for
the bus recovery function. The assumption is that the correct
pin-muxing has been taken care of by platform code or reset logic.
The pinctrl is only used to enter/exit bus recovery mode, nothing
else. Hence optional. So a typo in the pinctrl node only impacts bus
recovery function.
>
>> people fix problem that they don't really care by deliberately enlarge
>> the problem. That's why we don't panic() on any error we found. For
>> those who do care about the bus recovery, they can get the information
>> from the console.
>
> IMHO, it is just stupid to ignore errors and then let the developer
> later on trace back what the initial issue was. Error out early is a
> common sense software design principle...
>
> I am not asking for a panic(), I am just suggesting to only ignore
> pinctrl if it returns -ENODEV, the case you care are about.
It was just an analogy for enlarging the problem for getting
attention. But you probably thought that it was not enlarging the
problem as you think pinctrl is required by the driver instead of an
optional thing.
Regards,
Leo
^ permalink raw reply
* Re: [PATCH v5] i2c: imx: make bus recovery through pinctrl optional
From: Stefan Agner @ 2016-09-09 16:51 UTC (permalink / raw)
To: Leo Li
Cc: Uwe Kleine-König, Gao Pan, Wolfram Sang, lkml, Li Yang,
linux-gpio, linux-i2c, linux-arm-kernel
In-Reply-To: <CADRPPNR4oiRVgFqaM=1=Zn_JzrVJDVmozWsouLByXRtaHCh=qQ@mail.gmail.com>
On 2016-09-08 16:57, Leo Li wrote:
> On Thu, Sep 8, 2016 at 5:39 PM, Stefan Agner <stefan@agner.ch> wrote:
>> On 2016-09-06 15:40, Leo Li wrote:
>>> On Tue, Sep 6, 2016 at 4:51 PM, Stefan Agner <stefan@agner.ch> wrote:
>>>> On 2016-09-06 13:06, Leo Li wrote:
>>>>> On Tue, Sep 6, 2016 at 1:58 PM, Uwe Kleine-König
>>>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>>>> On Fri, Aug 19, 2016 at 05:05:22PM -0500, Li Yang wrote:
>> <snip>
>>>>>>> @@ -1081,8 +1090,11 @@ static int i2c_imx_probe(struct platform_device *pdev)
>>>>>>> return ret;
>>>>>>> }
>>>>>>>
>>>>>>> + /* optional bus recovery feature through pinctrl */
>>>>>>> i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
>>>>>>> - if (IS_ERR(i2c_imx->pinctrl)) {
>>>>>>> + /* bailout on -ENOMEM or -EPROBE_DEFER, continue for other errors */
>>>>>>> + if (PTR_ERR(i2c_imx->pinctrl) == -ENOMEM ||
>>>>>>> + PTR_ERR(i2c_imx->pinctrl) == -EPROBE_DEFER) {
>>>>>>> ret = PTR_ERR(i2c_imx->pinctrl);
>>>>>>> goto clk_disable;
>>>>>>> }
>>>>>>
>>>>>> devm_pinctrl_get might return the following error-valued pointers:
>>>>>> - -EINVAL
>>>>>> - -ENOMEM
>>>>>> - -ENODEV
>>>>>> - -EPROBE_DEFER
>>>>>>
>>>>>> There are several error paths returning -EINVAL, one is when an invalid
>>>>>> phandle is used. Do you really want to ignore that?
>>>>>>
>>>>>> IMO error handling is better done with inverse logic, that is continue
>>>>>> on some explicit error, bail out on all unknown stuff. This tends to be
>>>>>> more robust. Also the comment should be improved to not explain that for
>>>>>> -ENOMEM and -EPROBE_DEFER we bail out (which should be obvious for
>>>>>> anyone who can read C) but to explain why.
>>>>>
>>>>> What you said is true for normal error handling, but in this scenario
>>>>> it is intentional to ignore all pinctrl related errors except critical
>>>>> ones because failing to have pinctrl for an optional feature shouldn't
>>>>> impact the function of normal i2c. We choose to catch -ENOMEM because
>>>>> the error could also cause problem for i2c probe, and -EPROBE_DEFER
>>>>> because it's possible that the pinctrl will be ready later and we want
>>>>> to give it a chance. The i2c driver really don't care why the pinctrl
>>>>> was not usable. I thought I added comment before the
>>>>
>>>> I don't agree. E.g. -EINVAL would appear if you pass devm_pinctrl_get an
>>>> invalid device. Currently you would silently ignore that, which is not
>>>> what you want.
>>>
>>> It is not silently ignored, there will be a message printed out saying
>>> pinctrl is not available and bus recovery is not supported. On the
>>> contrary, without this change the entire i2c driver fails to work
>>> silently if pinctrl is somehow not working. And if the system is so
>>> broken that the pointer to the i2c device is NULL, the probe of i2c
>>> would have already failed before this point. We shouldn't count on an
>>> optional function of the driver to catch fundamental issues like this.
>>>
>>>>
>>>> You want to get the pinctrl in any case expect there isn't one. And that
>>>> is how you should formulate your if statement.
>>>>
>>>> /*
>>>> * It is ok if no pinctrl device is available. We'll not be able to use
>>>> the
>>>> * bus recovery feature, but otherwise the driver works fine...
>>>> */
>>>> if (PTR_ERR(i2c_imx->pinctrl) != -ENODEV)
>>>
>>> I agree that there could be other possibilities that the pinctrl
>>> failed to work beside the reason I described in the commit
>>> message(platform doesn't support pinctrl at all). But I don't think
>>> any of them other than the -ENOMEM and -EPROBE_DEFER deserves a bail
>>> out for the entire i2c driver.
>>
>> FWIW, I disagree. If there is pinctrl defined, you want be sure that it
>> gets applied properly, no matter what. E.g. when devm_pinctrl_get return
>> EINVAL (Uwe's example) the driver will continue and likely fail in
>> mysterious ways later on because the pins have not been muxed properly.
>> The driver should not load in that situation so that the developer is
>> forced to fix his mistakes. The only reason to bail out here is if there
>> is no pin controller (ENODEV). And it seems that Uwe also tends to that
>> solution.
>
> With this patch the i2c bus recovery feature will be disabled if the
> devm_pinctrl_get() fails. The pin mux setting will not be changed in
> either i2c probe stage or at runtime. I don't think it can cause any
> trouble to normal I2C operation. IMO, it is not good to *force*
If you have a pin controller, and you make a typo in your device tree
which leads to a wrong phandle and devm_pinctrl_get returning -EINVAL,
the system won't mux the pins... And that will certainly affect normal
I2C operation!
> people fix problem that they don't really care by deliberately enlarge
> the problem. That's why we don't panic() on any error we found. For
> those who do care about the bus recovery, they can get the information
> from the console.
IMHO, it is just stupid to ignore errors and then let the developer
later on trace back what the initial issue was. Error out early is a
common sense software design principle...
I am not asking for a panic(), I am just suggesting to only ignore
pinctrl if it returns -ENODEV, the case you care are about.
--
Stefan
^ permalink raw reply
* [patch v3] i2c: mux: mellanox: add driver
From: vadimp @ 2016-09-09 16:31 UTC (permalink / raw)
To: wsa, peda; +Cc: linux-i2c, linux-kernel, jiri, Vadim Pasternak, Michael Shych
From: Vadim Pasternak <vadimp@mellanox.com>
This driver allows I2C routing controlled through CPLD select registers on
wide range of Mellanox systems (CPLD Lattice device).
MUX selection is provided by digital and analog HW. Analog part is not
under SW control.
Digital part is under CPLD control (channel selection/de-selection).
Connectivity schema.
i2c-mlxcpld Digital Analog
driver
*--------* * -> mux1 (virt bus2) -> mux ->|
| I2CLPC | i2c physical * -> mux2 (virt bus3) -> mux ->|
| bridge | bus 1 *---------* |
| logic |---------------------> * mux reg * |
| in CPLD| *---------* |
*--------* i2c-mux-mlxpcld ^ * -> muxn (virt busn) -> mux ->|
| driver | |
| *---------------* | Devices
| * CPLD (i2c bus)* select |
| * registers for *--------*
| * mux selection * deselect
| *---------------*
| |
<--------> <----------->
i2c cntrl Board cntrl reg
reg space space (mux select,
IO, LED, WD, info)
i2c-mux-mlxpcld does not necessarily require i2c-mlxcpld. It can be use
along with another bus driver, and still control i2c routing through CPLD
mux selection, in case the system is equipped with CPLD capable of mux
selection control.
The Kconfig currently controlling compilation of this code is:
drivers/i2c/muxes/Kconfig:config I2C_MUX_MLXCPLD
Signed-off-by: Michael Shych <michaelsh@mellanox.com>
Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
v2->v3
Fixes issues pointed out by Peter:
- Fix several comments;
- In MAINTAINERS file use linux-i2c instead of linux-kernel;
- Place include files in alphabetic order;
- When channel select fail, set last_chan to zero for cleaning last value;
- Return with error from probe if there is no platform data;
- Use i2c_mux_reg driver for the lpc_access cases:
it fit well for Mellanox system with LPC access - has been tested and it
works good.
- Limit this driver to i2c_access only one device (mlxcpld_mux_module).
v1->v2
Fixes issues pointed out by Peter:
- changes in commit cover massage;
- change leg to channel in comments;
- missed return err;
- use platform data mux array rather the offset from 1st channel in probe
routine;
- remove owner field from driver structure;
- use module_i2c_driver macros, instead if __init and __exit;
- remove deselect on exit flag;
- add record to MAINTAINER file;
---
MAINTAINERS | 8 ++
drivers/i2c/muxes/Kconfig | 11 ++
drivers/i2c/muxes/Makefile | 1 +
drivers/i2c/muxes/i2c-mux-mlxcpld.c | 257 ++++++++++++++++++++++++++++++++++++
include/linux/i2c/mlxcpld.h | 57 ++++++++
5 files changed, 334 insertions(+)
create mode 100644 drivers/i2c/muxes/i2c-mux-mlxcpld.c
create mode 100644 include/linux/i2c/mlxcpld.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 0bbe4b1..be83d69 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7655,6 +7655,14 @@ W: http://www.mellanox.com
Q: http://patchwork.ozlabs.org/project/netdev/list/
F: drivers/net/ethernet/mellanox/mlxsw/
+MELLANOX MLXCPLD I2C MUX DRIVER
+M: Vadim Pasternak <vadimp@mellanox.com>
+M: Michael Shych <michaelsh@mellanox.com>
+L: linux-i2c@vger.kernel.org
+S: Supported
+W: http://www.mellanox.com
+F: drivers/i2c/muxes/i2c-mux-mlxcpld.c
+
SOFT-ROCE DRIVER (rxe)
M: Moni Shoua <monis@mellanox.com>
L: linux-rdma@vger.kernel.org
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index e280c8e..b7ab144 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -81,4 +81,15 @@ config I2C_DEMUX_PINCTRL
demultiplexer that uses the pinctrl subsystem. This is useful if you
want to change the I2C master at run-time depending on features.
+config I2C_MUX_MLXCPLD
+ tristate "Mellanox CPLD based I2C multiplexer"
+ help
+ If you say yes to this option, support will be included for a
+ CPLD based I2C multiplexer. This driver provides access to
+ I2C busses connected through a MUX, which is controlled
+ by a CPLD registers.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-mux-mlxcpld.
+
endmenu
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index 7c267c2..e5c990e 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -10,5 +10,6 @@ obj-$(CONFIG_I2C_MUX_PCA9541) += i2c-mux-pca9541.o
obj-$(CONFIG_I2C_MUX_PCA954x) += i2c-mux-pca954x.o
obj-$(CONFIG_I2C_MUX_PINCTRL) += i2c-mux-pinctrl.o
obj-$(CONFIG_I2C_MUX_REG) += i2c-mux-reg.o
+obj-$(CONFIG_I2C_MUX_MLXCPLD) += i2c-mux-mlxcpld.o
ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
diff --git a/drivers/i2c/muxes/i2c-mux-mlxcpld.c b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
new file mode 100644
index 0000000..0ca3d55
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
@@ -0,0 +1,257 @@
+/*
+ * drivers/i2c/muxes/i2c-mux-mlxcpld.c
+ * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the names of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/version.h>
+#include <linux/i2c/mlxcpld.h>
+
+#define CPLD_MUX_MAX_NCHANS 8
+
+/*
+ * mlxcpld_mux types - kind of mux supported by driver:
+ * @mlxcpld_mux_module - I2C access; 8 channels/legs.
+ */
+enum mlxcpld_mux_type {
+ mlxcpld_mux_module,
+};
+
+/* mlxcpld_mux - mux control structure:
+ * @type - mux type
+ * @last_chan - last register value
+ * @client - I2C device client
+ */
+struct mlxcpld_mux {
+ enum mlxcpld_mux_type type;
+ u8 last_chan;
+ struct i2c_client *client;
+};
+
+/* mlxcpld_mux_desc - mux descriptor structure:
+ * @nchans - number of channels
+ */
+struct mlxcpld_mux_desc {
+ u8 nchans;
+};
+
+/* MUX logic description.
+ * Driver can support different mux control logic, according to CPLD
+ * implementation.
+ *
+ * Connectivity schema.
+ *
+ * i2c-mlxcpld Digital Analog
+ * driver
+ * *--------* * -> mux1 (virt bus2) -> mux -> |
+ * | I2CLPC | i2c physical * -> mux2 (virt bus3) -> mux -> |
+ * | bridge | bus 1 *---------* |
+ * | logic |---------------------> * mux reg * |
+ * | in CPLD| *---------* |
+ * *--------* i2c-mux-mlxpcld ^ * -> muxn (virt busn) -> mux -> |
+ * | driver | |
+ * | *---------------* | Devices
+ * | * CPLD (i2c bus)* select |
+ * | * registers for *--------*
+ * | * mux selection * deselect
+ * | *---------------*
+ * | |
+ * <--------> <----------->
+ * i2c cntrl Board cntrl reg
+ * reg space space (mux select,
+ * IO, LED, WD, info)
+ *
+ */
+static const struct mlxcpld_mux_desc muxes[] = {
+ [mlxcpld_mux_module] = {
+ .nchans = CPLD_MUX_MAX_NCHANS,
+ },
+};
+
+static const struct i2c_device_id mlxcpld_mux_id[] = {
+ { "mlxcpld_mux_module", mlxcpld_mux_module },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, mlxcpld_mux_id);
+
+/* Write to mux register. Don't use i2c_transfer() and
+ * i2c_smbus_xfer() for this as they will try to lock adapter a second time
+ */
+static int mlxcpld_mux_reg_write(struct i2c_adapter *adap,
+ struct i2c_client *client, u8 val)
+{
+ struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client->dev);
+
+ if (adap->algo->master_xfer) {
+ struct i2c_msg msg;
+ u8 msgbuf[] = {pdata->sel_reg_addr, val};
+
+ msg.addr = pdata->addr;
+ msg.flags = 0;
+ msg.len = 2;
+ msg.buf = msgbuf;
+ return __i2c_transfer(adap, &msg, 1);
+ }
+ dev_err(&client->dev, "SMBus isn't supported on this adapter\n");
+
+ return -ENODEV;
+}
+
+static int mlxcpld_mux_select_chan(struct i2c_mux_core *muxc, u32 chan)
+{
+ struct mlxcpld_mux *data = i2c_mux_priv(muxc);
+ struct i2c_client *client = data->client;
+ u8 regval;
+ int err = 0;
+
+ if (data->type == mlxcpld_mux_module)
+ regval = chan + 1;
+ else
+ return -ENXIO;
+
+ /* Only select the channel if its different from the last channel */
+ if (data->last_chan != regval) {
+ err = mlxcpld_mux_reg_write(muxc->parent, client, regval);
+ if (err)
+ data->last_chan = 0;
+ else
+ data->last_chan = regval;
+ }
+
+ return err;
+}
+
+static int mlxcpld_mux_deselect(struct i2c_mux_core *muxc, u32 chan)
+{
+ struct mlxcpld_mux *data = i2c_mux_priv(muxc);
+ struct i2c_client *client = data->client;
+
+ /* Deselect active channel */
+ data->last_chan = 0;
+
+ return mlxcpld_mux_reg_write(muxc->parent, client, data->last_chan);
+}
+
+/* I2C init/probing/exit functions */
+static int mlxcpld_mux_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct i2c_adapter *adap = to_i2c_adapter(client->dev.parent);
+ struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client->dev);
+ struct i2c_mux_core *muxc;
+ int num, force;
+ u8 nchans;
+ struct mlxcpld_mux *data;
+ int err;
+
+ if (!pdata)
+ return -EINVAL;
+
+ if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE))
+ return -ENODEV;
+
+ switch (id->driver_data) {
+ case mlxcpld_mux_module:
+ nchans = muxes[id->driver_data].nchans;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ muxc = i2c_mux_alloc(adap, &client->dev, nchans, sizeof(*data), 0,
+ mlxcpld_mux_select_chan, mlxcpld_mux_deselect);
+ if (!muxc)
+ return -ENOMEM;
+
+ data = i2c_mux_priv(muxc);
+ i2c_set_clientdata(client, muxc);
+ data->client = client;
+ data->type = id->driver_data;
+ data->last_chan = 0; /* force the first selection */
+
+ /* Only in mlxcpld_mux_tor first_channel can be different.
+ * In other mlxcpld_mux types channel numbering begin from 1
+ * Now create an adapter for each channel
+ */
+ for (num = 0; num < muxes[data->type].nchans; num++) {
+ force = 0; /* dynamic adap number */
+ if (num < pdata->num_adaps)
+ force = pdata->adap_ids[num];
+ else
+ /* discard unconfigured channels */
+ break;
+
+ err = i2c_mux_add_adapter(muxc, force, num, 0);
+ if (err) {
+ dev_err(&client->dev, "failed to register multiplexed adapter %d as bus %d\n",
+ num, force);
+ goto virt_reg_failed;
+ }
+ }
+
+ return 0;
+
+virt_reg_failed:
+ i2c_mux_del_adapters(muxc);
+ return err;
+}
+
+static int mlxcpld_mux_remove(struct i2c_client *client)
+{
+ struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+
+ i2c_mux_del_adapters(muxc);
+ return 0;
+}
+
+static struct i2c_driver mlxcpld_mux_driver = {
+ .driver = {
+ .name = "mlxcpld-mux",
+ },
+ .probe = mlxcpld_mux_probe,
+ .remove = mlxcpld_mux_remove,
+ .id_table = mlxcpld_mux_id,
+};
+
+module_i2c_driver(mlxcpld_mux_driver);
+
+MODULE_AUTHOR("Michael Shych (michaels@mellanox.com)");
+MODULE_DESCRIPTION("Mellanox I2C-CPLD-MUX driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:i2c-mux-mlxcpld");
diff --git a/include/linux/i2c/mlxcpld.h b/include/linux/i2c/mlxcpld.h
new file mode 100644
index 0000000..1b46e5e
--- /dev/null
+++ b/include/linux/i2c/mlxcpld.h
@@ -0,0 +1,57 @@
+/*
+ * mlxcpld.h - Mellanox I2C multiplexer support in CPLD
+ *
+ * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the names of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _LINUX_I2C_MLXCPLD_H
+#define _LINUX_I2C_MLXCPLD_H
+
+/* Platform data for the CPLD I2C multiplexers */
+
+/* mlxcpld_mux_plat_data - per mux data, used with i2c_register_board_info
+ * @adap_ids - adapter array
+ * @num_adaps - number of adapters
+ * @sel_reg_addr - mux select register offset in CPLD space
+ * @first_channel - first channel to start virtual busses vector
+ * @addr - address of mux device - set to mux select register offset on LPC
+ * connected CPLDs or to I2C address on I2C conncted CPLDs
+ */
+struct mlxcpld_mux_plat_data {
+ int *adap_ids;
+ int num_adaps;
+ int sel_reg_addr;
+ int first_channel;
+ int addr;
+};
+
+#endif /* _LINUX_I2C_MLXCPLD_H */
--
2.1.4
^ permalink raw reply related
* [patch v1] i2c: add master driver for mellanox systems
From: vadimp @ 2016-09-09 15:04 UTC (permalink / raw)
To: wsa; +Cc: linux-i2c, linux-kernel, jiri, Vadim Pasternak, Michael Shych
From: Vadim Pasternak <vadimp@mellanox.com>
Device driver for Mellanox I2C controller logic, implemented in Lattice
CPLD device.
Device supports:
- Master mode
- One physical bus
- Polling mode
The Kconfig currently controlling compilation of this code is:
drivers/i2c/busses/Kconfig:config I2C_MLXCPLD
Signed-off-by: Michael Shych <michaelsh@mellanox.com>
Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
Documentation/i2c/busses/i2c-mlxcpld | 47 +++
MAINTAINERS | 9 +
drivers/i2c/busses/Kconfig | 12 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-mlxcpld.c | 597 +++++++++++++++++++++++++++++++++++
5 files changed, 666 insertions(+)
create mode 100644 Documentation/i2c/busses/i2c-mlxcpld
create mode 100644 drivers/i2c/busses/i2c-mlxcpld.c
diff --git a/Documentation/i2c/busses/i2c-mlxcpld b/Documentation/i2c/busses/i2c-mlxcpld
new file mode 100644
index 0000000..0f8678a
--- /dev/null
+++ b/Documentation/i2c/busses/i2c-mlxcpld
@@ -0,0 +1,47 @@
+Driver i2c-mlxcpld
+
+Author: Michael Shych <michaelsh@mellanox.com>
+
+This is a for Mellanox I2C controller logic, implemented in Lattice CPLD
+device.
+Device supports:
+ - Master mode.
+ - One physical bus.
+ - Polling mode.
+
+This controller is equipped within the next Mellanox systems:
+"msx6710", "msx6720", "msb7700", "msn2700", "msx1410", "msn2410", "msb7800",
+"msn2740", "msn2100".
+
+The next transaction types are supported:
+ - Receive Byte/Block.
+ - Send Byte/Block.
+ - Read Byte/Block.
+ - Write Byte/Block.
+
+Registers:
+CTRL 0x1 - control reg.
+ Resets all the registers.
+HALF_CYC 0x4 - cycle reg.
+ Configure the width of I2C SCL half clock cycle (in 4 LPC_CLK
+ units).
+I2C_HOLD 0x5 - hold reg.
+ OE (output enable) is delayed by value set to this register
+ (in LPC_CLK units)
+CMD 0x6 - command reg.
+ Bit 7(lsb), 0 = write, 1 = read.
+ Bits [6:0] - the 7bit Address of the I2C device.
+ It should be written last as it triggers an I2C transaction.
+NUM_DATA 0x7 - data size reg.
+ Number of address bytes to write in read transaction
+NUM_ADDR 0x8 - address reg.
+ Number of address bytes to write in read transaction.
+STATUS 0x9 - status reg.
+ Bit 0 - transaction is completed.
+ Bit 4 - ACK/NACK.
+DATAx 0xa - 0x54 - 68 bytes data buffer regs.
+ For write transaction address is specified in four first bytes
+ (DATA1 - DATA4), data starting from DATA4.
+ For read transactions address is send in separate transaction and
+ specified in four first bytes (DATA0 - DATA3). Data is reading
+ starting from DATA0.
diff --git a/MAINTAINERS b/MAINTAINERS
index 0bbe4b1..07fb0a1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7655,6 +7655,15 @@ W: http://www.mellanox.com
Q: http://patchwork.ozlabs.org/project/netdev/list/
F: drivers/net/ethernet/mellanox/mlxsw/
+MELLANOX MLXCPLD I2C DRIVER
+M: Vadim Pasternak <vadimp@mellanox.com>
+M: Michael Shych <michaelsh@mellanox.com>
+L: linux-i2c@vger.kernel.org
+S: Supported
+W: http://www.mellanox.com
+F: drivers/i2c/busses/i2c-mlxcpld.c
+F: Documentation/i2c/busses/i2c-mlxcpld
+
SOFT-ROCE DRIVER (rxe)
M: Moni Shoua <monis@mellanox.com>
L: linux-rdma@vger.kernel.org
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 5c3993b..1126142a 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1203,4 +1203,16 @@ config I2C_OPAL
This driver can also be built as a module. If so, the module will be
called as i2c-opal.
+config I2C_MLXCPLD
+ tristate "Mellanox I2C driver"
+ depends on X86_64
+ default y
+ help
+ This exposes the Mellanox platform I2C busses to the linux I2C layer
+ for X86 based systems.
+ Controller is implemented as CPLD logic.
+
+ This driver can also be built as a module. If so, the module will be
+ called as i2c-mlxcpld.
+
endmenu
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 37f2819..4df3578 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -118,5 +118,6 @@ obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o
obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o
obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o
obj-$(CONFIG_SCx200_ACB) += scx200_acb.o
+obj-$(CONFIG_I2C_MLXCPLD) += i2c-mlxcpld.o
ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
diff --git a/drivers/i2c/busses/i2c-mlxcpld.c b/drivers/i2c/busses/i2c-mlxcpld.c
new file mode 100644
index 0000000..dd62190
--- /dev/null
+++ b/drivers/i2c/busses/i2c-mlxcpld.c
@@ -0,0 +1,597 @@
+/*
+ * drivers/i2c/busses/i2c-mlxcpld.c
+ * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the names of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+/* General defines */
+#define MLXPLAT_CPLD_LPC_I2C_BASE_ADRR 0x2000
+#define MLXCPLD_I2C_DEVICE_NAME "i2c_mlxcpld"
+#define MLXCPLD_I2C_VALID_FLAG (I2C_M_RECV_LEN | I2C_M_RD)
+#define MLXCPLD_I2C_BUS_NUM 1
+#define MLXCPLD_I2C_DATA_REG_SZ 36
+#define MLXCPLD_I2C_MAX_ADDR_LEN 4
+#define MLXCPLD_I2C_RETR_NUM 2
+#define MLXCPLD_I2C_XFER_TO 500000 /* msec */
+#define MLXCPLD_I2C_POLL_TIME 2000 /* msec */
+
+/* LPC I2C registers */
+#define MLXCPLD_LPCI2C_LPF_REG 0x0
+#define MLXCPLD_LPCI2C_CTRL_REG 0x1
+#define MLXCPLD_LPCI2C_HALF_CYC_REG 0x4
+#define MLXCPLD_LPCI2C_I2C_HOLD_REG 0x5
+#define MLXCPLD_LPCI2C_CMD_REG 0x6
+#define MLXCPLD_LPCI2C_NUM_DAT_REG 0x7
+#define MLXCPLD_LPCI2C_NUM_ADDR_REG 0x8
+#define MLXCPLD_LPCI2C_STATUS_REG 0x9
+#define MLXCPLD_LPCI2C_DATA_REG 0xa
+
+/* LPC I2C masks and parametres */
+#define MLXCPLD_LPCI2C_RST_SEL_MASK 0x1
+#define MLXCPLD_LPCI2C_LPF_DFLT 0x2
+#define MLXCPLD_LPCI2C_HALF_CYC_100 0x1f
+#define MLXCPLD_LPCI2C_I2C_HOLD_100 0x3c
+#define MLXCPLD_LPCI2C_TRANS_END 0x1
+#define MLXCPLD_LPCI2C_STATUS_NACK 0x10
+#define MLXCPLD_LPCI2C_ERR_IND -1
+#define MLXCPLD_LPCI2C_NO_IND 0
+#define MLXCPLD_LPCI2C_ACK_IND 1
+#define MLXCPLD_LPCI2C_NACK_IND 2
+
+/**
+ * mlxcpld_i2c_regs - controller registers:
+ * @half_cyc - half cycle register
+ * @i2c_hold - hold register
+ * @config - config register
+ * @cmd - command register
+ * @cmd - status register
+ * @data - register data
+**/
+struct mlxcpld_i2c_regs {
+ u8 half_cyc;
+ u8 i2c_hold;
+ u8 config;
+ u8 cmd;
+ u8 status;
+ u8 data[MLXCPLD_I2C_DATA_REG_SZ];
+};
+
+/**
+ * mlxcpld_i2c_curr_transf - current transaction parameters:
+ * @cmd - command
+ * @addr_width - address width
+ * @data_len - data length
+ * @cmd - command register
+ * @msg_num - message number
+ * @msg - pointer to message buffer
+**/
+struct mlxcpld_i2c_curr_transf {
+ u8 cmd;
+ u8 addr_width;
+ u8 data_len;
+ u8 msg_num;
+ struct i2c_msg *msg;
+};
+
+/**
+ * mlxcpld_i2c_priv - private controller data:
+ * @lpc_gen_dec_reg - register space
+ * @adap - i2c adapter
+ * @dev_id - device id
+ * @base_addr - base IO address
+ * @poll_time - polling time
+ * @xfer_to - transfer timeout in microsec (500000)
+ * @retr_num - access retries number (2)
+ * @block_sz - maximum data block size (36),
+ * @lock - bus access lock
+ * @lpc_i2c_res - lpc i2c resourse
+ * @lpc_cpld_res - lpc cpld resource
+ * @xfer - current i2c transfer block
+ * @pdev - platform device
+**/
+struct mlxcpld_i2c_priv {
+ struct i2c_adapter adap;
+ u16 dev_id;
+ u16 base_addr;
+ u16 poll_time;
+ int xfer_to;
+ int retr_num;
+ int block_sz;
+ struct mutex lock;
+ struct mlxcpld_i2c_curr_transf xfer;
+ struct platform_device *pdev;
+};
+struct platform_device *mlxcpld_i2c_plat_dev;
+
+static void mlxcpld_i2c_lpc_write_buf(u8 *data, u8 len, u32 addr)
+{
+ int i, nbyte, ndword;
+
+ nbyte = len % 4;
+ ndword = len / 4;
+ for (i = 0; i < ndword; i++)
+ outl(*((u32 *)data + i), addr + i * 4);
+ ndword *= 4;
+ addr += ndword;
+ data += ndword;
+ for (i = 0; i < nbyte; i++)
+ outb(*(data + i), addr + i);
+}
+
+static void mlxcpld_i2c_lpc_read_buf(u8 *data, u8 len, u32 addr)
+{
+ int i, nbyte, ndword;
+
+ nbyte = len % 4;
+ ndword = len / 4;
+ for (i = 0; i < ndword; i++)
+ *((u32 *)data + i) = inl(addr + i * 4);
+ ndword *= 4;
+ addr += ndword;
+ data += ndword;
+ for (i = 0; i < nbyte; i++)
+ *(data + i) = inb(addr + i);
+}
+
+static void mlxcpld_i2c_read_comm(struct mlxcpld_i2c_priv *priv, u8 offs,
+ u8 *data, u8 datalen)
+{
+ u32 addr = priv->base_addr + offs;
+
+ switch (datalen) {
+ case 1:
+ *(data) = inb(addr);
+ break;
+ case 2:
+ *((u16 *)data) = inw(addr);
+ break;
+ case 3:
+ *((u16 *)data) = inw(addr);
+ *(data + 2) = inb(addr + 2);
+ break;
+ case 4:
+ *((u32 *)data) = inl(addr);
+ break;
+ default:
+ mlxcpld_i2c_lpc_read_buf(data, datalen, addr);
+ break;
+ }
+}
+
+static void mlxcpld_i2c_write_comm(struct mlxcpld_i2c_priv *priv, u8 offs,
+ u8 *data, u8 datalen)
+{
+ u32 addr = priv->base_addr + offs;
+
+ switch (datalen) {
+ case 1:
+ outb(*(data), addr);
+ break;
+ case 2:
+ outw(*((u16 *)data), addr);
+ break;
+ case 3:
+ outw(*((u16 *)data), addr);
+ outb(*(data + 2), addr + 2);
+ break;
+ case 4:
+ outl(*((u32 *)data), addr);
+ break;
+ default:
+ mlxcpld_i2c_lpc_write_buf(data, datalen, addr);
+ break;
+ }
+}
+
+/* Check validity of current i2c message and all transfer.
+ * Calculate also coomon length of all i2c messages in transfer.
+ */
+static int mlxcpld_i2c_invalid_len(struct mlxcpld_i2c_priv *priv,
+ const struct i2c_msg *msg, u8 *comm_len)
+{
+ u8 max_len = msg->flags == I2C_M_RD ? priv->block_sz -
+ MLXCPLD_I2C_MAX_ADDR_LEN : priv->block_sz;
+
+ if (msg->len < 0 || msg->len > max_len)
+ return -EINVAL;
+
+ *comm_len += msg->len;
+ if (*comm_len > priv->block_sz)
+ return -EINVAL;
+ else
+ return 0;
+}
+
+/* Check validity of received i2c messages parameters.
+ * Returns 0 if OK, other - in case of invalid parameters
+ * or common length of data that should be passed to CPLD
+ */
+static int mlxcpld_i2c_check_msg_params(struct mlxcpld_i2c_priv *priv,
+ struct i2c_msg *msgs, int num,
+ u8 *comm_len)
+{
+ int i;
+
+ if (!num) {
+ dev_err(&priv->pdev->dev, "Incorrect 0 num of messages\n");
+ return -EINVAL;
+ }
+
+ if (unlikely(msgs[0].addr > 0x7f)) {
+ dev_err(&priv->pdev->dev, "Invalid address 0x%03x\n",
+ msgs[0].addr);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < num; ++i) {
+ if (unlikely(!msgs[i].buf)) {
+ dev_err(&priv->pdev->dev, "Invalid buf in msg[%d]\n",
+ i);
+ return -EINVAL;
+ }
+ if (unlikely(msgs[0].addr != msgs[i].addr)) {
+ dev_err(&priv->pdev->dev, "Invalid addr in msg[%d]\n",
+ i);
+ return -EINVAL;
+ }
+ if (unlikely(mlxcpld_i2c_invalid_len(priv, &msgs[i],
+ comm_len))) {
+ dev_err(&priv->pdev->dev, "Invalid len %d msg[%d], addr 0x%x, lag %u\n",
+ msgs[i].len, i, msgs[i].addr, msgs[i].flags);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+/* Check if transfer is completed and status of operation.
+ * Returns 0 - transfer completed (both ACK or NACK),
+ * negative - transfer isn't finished.
+ */
+static int mlxcpld_i2c_check_status(struct mlxcpld_i2c_priv *priv, int *status)
+{
+ u8 val;
+
+ mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_STATUS_REG, &val, 1);
+
+ if (val & MLXCPLD_LPCI2C_TRANS_END) {
+ if (val & MLXCPLD_LPCI2C_STATUS_NACK)
+ /* The slave is unable to accept the data. No such
+ * slave, command not understood, or unable to accept
+ * any more data.
+ */
+ *status = MLXCPLD_LPCI2C_NACK_IND;
+ else
+ *status = MLXCPLD_LPCI2C_ACK_IND;
+ return 0;
+ }
+ *status = MLXCPLD_LPCI2C_NO_IND;
+
+ return -EIO;
+}
+
+static void mlxcpld_i2c_set_transf_data(struct mlxcpld_i2c_priv *priv,
+ struct i2c_msg *msgs, int num,
+ u8 comm_len)
+{
+ priv->xfer.msg = msgs;
+ priv->xfer.msg_num = num;
+
+ /*
+ * All upper layers currently are never use transfer with more than
+ * 2 messages. Actually, it's also not so relevant in Mellanox systems
+ * because of HW limitation. Max size of transfer is o more than 20B
+ * in current x86 LPCI2C bridge.
+ */
+ priv->xfer.cmd = (msgs[num - 1].flags & I2C_M_RD);
+
+ if (priv->xfer.cmd == I2C_M_RD) {
+ if (comm_len == msgs[0].len) {
+ /* Special case of addr_width = 0 */
+ priv->xfer.addr_width = 0;
+ priv->xfer.data_len = comm_len;
+ } else {
+ priv->xfer.addr_width = msgs[0].len;
+ priv->xfer.data_len = comm_len - priv->xfer.addr_width;
+ }
+ } else {
+ /* Width (I2C_NUM_ADDR reg) isn't used in write command. */
+ priv->xfer.addr_width = 0;
+ priv->xfer.data_len = comm_len;
+ }
+}
+
+/* Reset CPLD LPCI2C block */
+static void mlxcpld_i2c_reset(struct mlxcpld_i2c_priv *priv)
+{
+ u8 val;
+
+ mutex_lock(&priv->lock);
+ mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_CTRL_REG, &val, 1);
+ val &= ~MLXCPLD_LPCI2C_RST_SEL_MASK;
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_CTRL_REG, &val, 1);
+ mutex_unlock(&priv->lock);
+}
+
+/* Make sure the CPLD is ready to start transmitting.
+ * Return 0 if it is, -EBUSY if it is not.
+ */
+static int mlxcpld_i2c_check_busy(struct mlxcpld_i2c_priv *priv)
+{
+ u8 val;
+
+ mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_STATUS_REG, &val, 1);
+
+ if (val & MLXCPLD_LPCI2C_TRANS_END)
+ return 0;
+
+ return -EIO;
+}
+
+static int mlxcpld_i2c_wait_for_free(struct mlxcpld_i2c_priv *priv)
+{
+ int timeout = 0;
+
+ do {
+ if (!mlxcpld_i2c_check_busy(priv))
+ break;
+ usleep_range(priv->poll_time/2, priv->poll_time);
+ timeout += priv->poll_time;
+ } while (timeout < priv->xfer_to);
+
+ if (timeout > priv->xfer_to)
+ return -ETIMEDOUT;
+
+ return 0;
+}
+
+/*
+ * Wait for master transfer to complete.
+ * It puts current process to sleep until we get interrupt or timeout expires.
+ * Returns the number of transferred or read bytes or error (<0).
+ */
+static int mlxcpld_i2c_wait_for_tc(struct mlxcpld_i2c_priv *priv)
+{
+ int status, i = 1, timeout = 0;
+ u8 datalen;
+ int err = 0;
+
+ do {
+ usleep_range(priv->poll_time / 2, priv->poll_time);
+ if (!mlxcpld_i2c_check_status(priv, &status))
+ break;
+ timeout += priv->poll_time;
+ } while (status == 0 && timeout < priv->xfer_to);
+
+ switch (status) {
+ case MLXCPLD_LPCI2C_NO_IND:
+ return -ETIMEDOUT;
+ case MLXCPLD_LPCI2C_ACK_IND:
+ if (priv->xfer.cmd == I2C_M_RD) {
+ /*
+ * Actual read data len will be always the same as
+ * requested len. 0xff (line pull-up) will be returned
+ * if slave has no data to return. Thus don't read
+ * MLXCPLD_LPCI2C_NUM_DAT_REG reg from CPLD.
+ */
+ err = datalen = priv->xfer.data_len;
+ if (priv->xfer.msg_num == 1)
+ i = 0;
+
+ if (!priv->xfer.msg[i].buf)
+ err = -EINVAL;
+ else
+ mlxcpld_i2c_read_comm(priv,
+ MLXCPLD_LPCI2C_DATA_REG,
+ priv->xfer.msg[i].buf,
+ datalen);
+ } else {
+ err = priv->xfer.addr_width + priv->xfer.data_len;
+ }
+ break;
+ case MLXCPLD_LPCI2C_NACK_IND:
+ err = -EAGAIN;
+ break;
+ case MLXCPLD_LPCI2C_ERR_IND:
+ err = -EIO;
+ break;
+ default:
+ break;
+ }
+
+ return err;
+}
+
+static void mlxcpld_i2c_xfer_msg(struct mlxcpld_i2c_priv *priv)
+{
+ int i, len = 0;
+ u8 cmd;
+
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_NUM_DAT_REG,
+ &priv->xfer.data_len, 1);
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_NUM_ADDR_REG,
+ &priv->xfer.addr_width, 1);
+
+ for (i = 0; i < priv->xfer.msg_num; i++) {
+ if ((priv->xfer.msg[i].flags & I2C_M_RD) != I2C_M_RD) {
+ /* Don't write to CPLD buffer in read transaction */
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_DATA_REG +
+ len, priv->xfer.msg[i].buf,
+ priv->xfer.msg[i].len);
+ len += priv->xfer.msg[i].len;
+ }
+ }
+
+ /* Set target slave address with command for master transfer.
+ * It should be latest executed function before CPLD transaction.
+ */
+ cmd = (priv->xfer.msg[0].addr << 1) | priv->xfer.cmd;
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_CMD_REG, &cmd, 1);
+}
+
+/* Generic lpc-i2c transfer.
+ * Returns the number of processed messages or error (<0).
+ */
+static int mlxcpld_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
+ int num)
+{
+ struct mlxcpld_i2c_priv *priv = i2c_get_adapdata(adap);
+ u8 comm_len = 0;
+ int err;
+
+ err = mlxcpld_i2c_check_msg_params(priv, msgs, num, &comm_len);
+ if (err) {
+ dev_err(&priv->pdev->dev, "Incorrect message\n");
+ return err;
+ }
+
+ /* Check bus state */
+ if (mlxcpld_i2c_wait_for_free(priv)) {
+ dev_err(&priv->pdev->dev, "LPCI2C bridge is busy\n");
+
+ /*
+ * Usually it means something serious has happened.
+ * We can not have unfinished previous transfer
+ * so it doesn't make any sense to try to stop it.
+ * Probably we were not able to recover from the
+ * previous error.
+ * The only reasonable thing - is soft reset.
+ */
+ mlxcpld_i2c_reset(priv);
+ if (mlxcpld_i2c_check_busy(priv)) {
+ dev_err(&priv->pdev->dev, "LPCI2C bridge is busy after reset\n");
+ return -EIO;
+ }
+ }
+
+ mlxcpld_i2c_set_transf_data(priv, msgs, num, comm_len);
+
+ mutex_lock(&priv->lock);
+ /* Do real transfer. Can't fail */
+ mlxcpld_i2c_xfer_msg(priv);
+ /* Wait for transaction complete */
+ err = mlxcpld_i2c_wait_for_tc(priv);
+ mutex_unlock(&priv->lock);
+
+ return err < 0 ? err : num;
+}
+
+static u32 mlxcpld_i2c_func(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+static const struct i2c_algorithm mlxcpld_i2c_algo = {
+ .master_xfer = mlxcpld_i2c_xfer,
+ .functionality = mlxcpld_i2c_func
+};
+
+static struct i2c_adapter mlxcpld_i2c_adapter = {
+ .owner = THIS_MODULE,
+ .name = "i2c-mlxcpld",
+ .class = I2C_CLASS_HWMON | I2C_CLASS_SPD,
+ .algo = &mlxcpld_i2c_algo,
+};
+
+static int mlxcpld_i2c_probe(struct platform_device *pdev)
+{
+ struct mlxcpld_i2c_priv *priv;
+ int err;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(struct mlxcpld_i2c_priv),
+ GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ mutex_init(&priv->lock);
+ platform_set_drvdata(pdev, priv);
+ priv->pdev = pdev;
+ priv->xfer_to = MLXCPLD_I2C_XFER_TO;
+ priv->retr_num = MLXCPLD_I2C_RETR_NUM;
+ priv->block_sz = MLXCPLD_I2C_DATA_REG_SZ;
+ priv->poll_time = MLXCPLD_I2C_POLL_TIME;
+ /* Register with i2c layer */
+ priv->adap = mlxcpld_i2c_adapter;
+ priv->adap.dev.parent = &pdev->dev;
+ i2c_set_adapdata(&priv->adap, priv);
+ priv->adap.retries = priv->retr_num;
+ priv->adap.nr = MLXCPLD_I2C_BUS_NUM;
+ priv->adap.timeout = usecs_to_jiffies(priv->xfer_to);
+
+ err = i2c_add_numbered_adapter(&priv->adap);
+ if (err) {
+ dev_err(&pdev->dev, "Failed to add %s adapter (%d)\n",
+ MLXCPLD_I2C_DEVICE_NAME, err);
+ goto fail_adapter;
+ }
+
+ priv->base_addr = MLXPLAT_CPLD_LPC_I2C_BASE_ADRR;
+
+ return 0;
+
+fail_adapter:
+ mutex_destroy(&priv->lock);
+ return err;
+}
+
+static int mlxcpld_i2c_remove(struct platform_device *pdev)
+{
+ struct mlxcpld_i2c_priv *priv = platform_get_drvdata(pdev);
+
+ i2c_del_adapter(&priv->adap);
+ mutex_destroy(&priv->lock);
+
+ return 0;
+}
+
+static struct platform_driver mlxcpld_i2c_driver = {
+ .probe = mlxcpld_i2c_probe,
+ .remove = mlxcpld_i2c_remove,
+ .driver = {
+ .name = MLXCPLD_I2C_DEVICE_NAME,
+ },
+};
+
+module_platform_driver(mlxcpld_i2c_driver);
+
+MODULE_AUTHOR("Michael Shych (michaels@mellanox.com)");
+MODULE_DESCRIPTION("Mellanox I2C-CPLD controller driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:i2c-mlxcpld");
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] i2c: mux: demux-pinctrl: run properly with multiple instances
From: Wolfram Sang @ 2016-09-09 12:20 UTC (permalink / raw)
To: Simon Horman
Cc: Wolfram Sang, linux-i2c, linux-renesas-soc, Pantelis Antoniou
In-Reply-To: <20160909070844.GB18476@verge.net.au>
[-- Attachment #1: Type: text/plain, Size: 254 bytes --]
> > > Simon: I think you can retest your demuxer-dts-series now.
> >
> > Simon: you think we can get that into 4.9? Would be awesome.
>
> Sorry, this slipped through the cracks.
> Lets aim for v4.10.
Awwww.... ;) Ok, will update the todo.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] i2c: mux: demux-pinctrl: run properly with multiple instances
From: Simon Horman @ 2016-09-09 7:08 UTC (permalink / raw)
To: Wolfram Sang
Cc: Wolfram Sang, linux-i2c, linux-renesas-soc, Pantelis Antoniou
In-Reply-To: <20160908145027.GB1485@katana>
On Thu, Sep 08, 2016 at 04:50:27PM +0200, Wolfram Sang wrote:
> On Tue, Aug 23, 2016 at 05:28:03PM +0200, Wolfram Sang wrote:
> > We can't use a static property for all the changesets, so we now create
> > dynamic ones for each changeset.
> >
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > Fixes: 50a5ba87690814 ("i2c: mux: demux-pinctrl: add driver")
>
> Applied to for-current (and thus will be in 4.8), thanks!
>
> > Simon: I think you can retest your demuxer-dts-series now.
>
> Simon: you think we can get that into 4.9? Would be awesome.
Sorry, this slipped through the cracks.
Lets aim for v4.10.
^ permalink raw reply
* Re: [v11, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
From: Scott Wood @ 2016-09-09 3:47 UTC (permalink / raw)
To: Yangbo Lu, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, Arnd Bergmann
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Russell King,
Bhupesh Sharma, netdev-u79uwXL29TY76Z2rM5mHXA, Santosh Shilimkar,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Jochen Friedrich,
xiaobo.xie-3arQi8VN3Tc,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Claudiu Manoil, Kumar Gala,
leoyang.li-3arQi8VN3Tc, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Qiang Zhao
In-Reply-To: <1473150503-9550-6-git-send-email-yangbo.lu-3arQi8VN3Tc@public.gmane.org>
On Tue, 2016-09-06 at 16:28 +0800, Yangbo Lu wrote:
> The global utilities block controls power management, I/O device
> enabling, power-onreset(POR) configuration monitoring, alternate
> function selection for multiplexed signals,and clock control.
>
> This patch adds a driver to manage and access global utilities block.
> Initially only reading SVR and registering soc device are supported.
> Other guts accesses, such as reading RCW, should eventually be moved
> into this driver as well.
>
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> Signed-off-by: Scott Wood <oss@buserror.net>
Don't put my signoff on patches that I didn't put it on myself. Definitely
don't put mine *after* yours on patches that were last modified by you.
If you want to mention that the soc_id encoding was my suggestion, then do so
explicitly.
> +/* SoC attribute definition for QorIQ platform */
> +static const struct soc_device_attribute qoriq_soc[] = {
> +#ifdef CONFIG_PPC
> + /*
> + * Power Architecture-based SoCs T Series
> + */
> +
> + /* SoC: T1024/T1014/T1023/T1013 Rev: 1.0 */
> + { .soc_id = "svr:0x85400010,name:T1024,die:T1024",
> + .revision = "1.0",
> + },
> + { .soc_id = "svr:0x85480010,name:T1024E,die:T1024",
> + .revision = "1.0",
> + },
Revision could be computed from the low 8 bits of SVR (just as you do for unknown SVRs).
We could move the die name into .family:
{
.soc_id = "svr:0x85490010,name:T1023E,",
.family = "QorIQ T1024",
}
I see you dropped svre (and the trailing comma), though I guess the vast
majority of potential users will be looking at .family. In which case do we
even need name? If we just make the soc_id be "svr:0xnnnnnnnn" then we could
shrink the table to an svr+mask that identifies each die. I'd still want to
keep the "svr:" even if we're giving up on the general tagging system, to make
it clear what the number refers to, and to provide some defense against users
who match only against soc_id rather than soc_id+family. Or we could go
further and format soc_id as "QorIQ SVR 0xnnnnnnnn" so that soc_id-only
matches are fully acceptable rather than just less dangerous.
> +static const struct soc_device_attribute *fsl_soc_device_match(
> + unsigned int svr, const struct soc_device_attribute *matches)
> +{
> + char svr_match[50];
> + int n;
> +
> + n = sprintf(svr_match, "*%08x*", svr);
n = sprintf(svr_match, "svr:0x%08x,*", svr);
(according to the current encoding)
> +
> + do {
> + if (!matches->soc_id)
> + return NULL;
> + if (glob_match(svr_match, matches->soc_id))
> + break;
> + } while (matches++);
Are you expecting "matches++" to ever evaluate as false?
> + /* Register soc device */
> + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> + if (!soc_dev_attr) {
> + ret = -ENOMEM;
> + goto out_unmap;
> + }
Couldn't this be statically allocated?
> +
> + machine = of_flat_dt_get_machine_name();
> + if (machine)
> + soc_dev_attr->machine = kasprintf(GFP_KERNEL, "%s",
> machine);
> +
> + soc_dev_attr->family = kasprintf(GFP_KERNEL, "QorIQ");
> +
> + svr = fsl_guts_get_svr();
> + fsl_soc = fsl_soc_device_match(svr, qoriq_soc);
> + if (fsl_soc) {
> + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%s",
> + fsl_soc->soc_id);
You can use kstrdup() if you're just copying the string as is.
> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%s",
> + fsl_soc->revision);
> + } else {
> + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "0x%08x",
> svr);
kasprintf(GFP_KERNEL, "svr:0x%08x,", svr);
> +
> + soc_dev = soc_device_register(soc_dev_attr);
> + if (IS_ERR(soc_dev)) {
> + ret = -ENODEV;
Why are you changing the error code?
> + goto out;
> + } else {
Unnecessary "else".
> + pr_info("Detected: %s\n", soc_dev_attr->machine);
Machine: %s
> + pr_info("Detected SoC family: %s\n", soc_dev_attr->family);
> + pr_info("Detected SoC ID: %s, revision: %s\n",
> + soc_dev_attr->soc_id, soc_dev_attr->revision);
s/Detected //g
> + }
> + return 0;
> +out:
> + kfree(soc_dev_attr->machine);
> + kfree(soc_dev_attr->family);
> + kfree(soc_dev_attr->soc_id);
> + kfree(soc_dev_attr->revision);
> + kfree(soc_dev_attr);
> +out_unmap:
> + iounmap(guts->regs);
> +out_free:
> + kfree(guts);
devm
> +static int fsl_guts_remove(struct platform_device *dev)
> +{
> + kfree(soc_dev_attr->machine);
> + kfree(soc_dev_attr->family);
> + kfree(soc_dev_attr->soc_id);
> + kfree(soc_dev_attr->revision);
> + kfree(soc_dev_attr);
> + soc_device_unregister(soc_dev);
> + iounmap(guts->regs);
> + kfree(guts);
> + return 0;
> +}
Don't free the memory before you unregister the device that uses it (moot if
you use devm).
>
> +#ifdef CONFIG_FSL_GUTS
> +unsigned int fsl_guts_get_svr(void);
> +#endif
Don't ifdef prototypes (unless you're going to provide a stub alternative).
-Scott
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply
* RE: [Patch V3] i2c: imx: add low power i2c bus driver
From: Pandy Gao @ 2016-09-09 1:49 UTC (permalink / raw)
To: wsa@the-dreams.de, u.kleine-koenig@pengutronix.de,
cmo@melexis.com
Cc: linux-i2c@vger.kernel.org, Frank Li, Andy Duan
In-Reply-To: <1471420784-11727-1-git-send-email-pandy.gao@nxp.com>
Ping...
> -----Original Message-----
> From: Pan Gao
> Sent: Wednesday, August 17, 2016 4:00 PM
> To: wsa@the-dreams.de; u.kleine-koenig@pengutronix.de; cmo@melexis.com
> Cc: linux-i2c@vger.kernel.org; Frank Li <frank.li@nxp.com>; Fugang Duan
> <fugang.duan@nxp.com>; Pan Gao <pandy.gao@nxp.com>
> Subject: [Patch V3] i2c: imx: add low power i2c bus driver
>
> This patch adds low power i2c bus driver to support new i.MX products which use
> low power i2c instead of the old imx i2c.
>
> The low power i2c can continue operating in stop mode when an appropriate
> clock is available. It is also designed for low CPU overhead with DMA offloading of
> FIFO register accesses.
>
> Signed-off-by: Gao Pan <pandy.gao@nxp.com>
> Reviewed-by: Fugang Duan <B38611@freescale.com>
> ---
> V2:
> -stop i2c transfer under the wrong condition -add timeout check in while()
> domain
>
> V3:
> -fix typo inside commit message and the driver.
>
> .../devicetree/bindings/i2c/i2c-imx-lpi2c.txt | 25 +
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-imx-lpi2c.c | 667 +++++++++++++++++++++
> 4 files changed, 703 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> new file mode 100644
> index 0000000..1f10cbf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> @@ -0,0 +1,25 @@
> +* Freescale Low Power Inter IC (LPI2C) for i.MX
> +
> +Required properties:
> +- compatible :
> + - "fsl,imx8dv-lpi2c" for LPI2C compatible with the one integrated on
> +i.MX8DV soc
> + - "fsl,imx7ulp-lpi2c" for LPI2C compatible with the one integrated on
> +i.MX7ULP soc
> +- reg : Should contain LPI2C registers location and length
> +- interrupts : Should contain LPI2C interrupt
> +- clocks : Should contain LPI2C clock specifier
> +- power-domains : should contain LPI2C power domain
> +
> +Optional properties:
> +- clock-frequency : Constains desired LPI2C bus clock frequency in Hz.
> + The absence of the property indicates the default frequency 100 kHz.
> +
> +Examples:
> +
> +i2c1: i2c@5e110000 { /* LPI2C on i.MX8DV */
> + compatible = "fsl,imx8dv-lpi2c";
> + reg = <0x0 0x5e110000 0x0 0x4000>;
> + interrupts = <0 88 4>;
> + clocks = <&clk IMX8DV_I2C1_CLK>;
> + clock-names = "per";
> + power-domains = <&pd_lsio_i2c1>;
> +};
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index
> efa3d9b..1fc7a10 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -596,6 +596,16 @@ config I2C_IMX
> This driver can also be built as a module. If so, the module
> will be called i2c-imx.
>
> +config I2C_IMX_LPI2C
> + tristate "IMX Low Power I2C interface"
> + depends on ARCH_MXC || COMPILE_TEST
> + help
> + Say Y here if you want to use the Low Power IIC bus controller
> + on the Freescale i.MX processors.
> +
> + This driver can also be built as a module. If so, the module
> + will be called i2c-imx-lpi2c.
> +
> config I2C_IOP3XX
> tristate "Intel IOPx3xx and IXP4xx on-chip I2C interface"
> depends on ARCH_IOP32X || ARCH_IOP33X || ARCH_IXP4XX ||
> ARCH_IOP13XX diff --git a/drivers/i2c/busses/Makefile
> b/drivers/i2c/busses/Makefile index 37f2819..cc93457 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_I2C_HIX5HD2) += i2c-hix5hd2.o
> obj-$(CONFIG_I2C_IBM_IIC) += i2c-ibm_iic.o
> obj-$(CONFIG_I2C_IMG) += i2c-img-scb.o
> obj-$(CONFIG_I2C_IMX) += i2c-imx.o
> +obj-$(CONFIG_I2C_IMX_LPI2C) += i2c-imx-lpi2c.o
> obj-$(CONFIG_I2C_IOP3XX) += i2c-iop3xx.o
> obj-$(CONFIG_I2C_JZ4780) += i2c-jz4780.o
> obj-$(CONFIG_I2C_KEMPLD) += i2c-kempld.o
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> new file mode 100644
> index 0000000..308ecf5
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -0,0 +1,667 @@
> +/*
> + * This is i.MX low power i2c controller driver.
> + *
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +
> +#define DRIVER_NAME "imx-lpi2c"
> +
> +#define LPI2C_PARAM 0x04 /* i2c RX/TX FIFO size */
> +#define LPI2C_MCR 0x10 /* i2c contrl register */
> +#define LPI2C_MSR 0x14 /* i2c status register */
> +#define LPI2C_MIER 0x18 /* i2c interrupt enable */
> +#define LPI2C_MCFGR0 0x20 /* i2c master configuration */
> +#define LPI2C_MCFGR1 0x24 /* i2c master configuration */
> +#define LPI2C_MCFGR2 0x28 /* i2c master configuration */
> +#define LPI2C_MCFGR3 0x2C /* i2c master configuration */
> +#define LPI2C_MCCR0 0x48 /* i2c master clk configuration */
> +#define LPI2C_MCCR1 0x50 /* i2c master clk configuration */
> +#define LPI2C_MFCR 0x58 /* i2c master FIFO control */
> +#define LPI2C_MFSR 0x5C /* i2c master FIFO status */
> +#define LPI2C_MTDR 0x60 /* i2c master TX data register */
> +#define LPI2C_MRDR 0x70 /* i2c master RX data register */
> +
> +/* i2c command */
> +#define TRAN_DATA 0X00
> +#define RECV_DATA 0X01
> +#define GEN_STOP 0X02
> +#define RECV_DISCARD 0X03
> +#define GEN_START 0X04
> +#define START_NACK 0X05
> +#define START_HIGH 0X06
> +#define START_HIGH_NACK 0X07
> +
> +#define MCR_MEN (1 << 0)
> +#define MCR_RST (1 << 1)
> +#define MCR_DOZEN (1 << 2)
> +#define MCR_DBGEN (1 << 3)
> +#define MCR_RTF (1 << 8)
> +#define MCR_RRF (1 << 9)
> +#define MSR_TDF (1 << 0)
> +#define MSR_RDF (1 << 1)
> +#define MSR_SDF (1 << 9)
> +#define MSR_NDF (1 << 10)
> +#define MSR_ALF (1 << 11)
> +#define MSR_MBF (1 << 24)
> +#define MSR_BBF (1 << 25)
> +#define MIER_TDIE (1 << 0)
> +#define MIER_RDIE (1 << 1)
> +#define MIER_SDIE (1 << 9)
> +#define MIER_NDIE (1 << 10)
> +#define MCFGR1_AUTOSTOP (1 << 8)
> +#define MCFGR1_IGNACK (1 << 9)
> +#define MRDR_RXEMPTY (1 << 14)
> +
> +#define I2C_CLK_RATIO 2
> +#define CHUNK_DATA 256
> +
> +#define LPI2C_RX_FIFOSIZE 4
> +#define LPI2C_TX_FIFOSIZE 4
> +
> +#define LPI2C_DEFAULT_RATE 100000
> +#define STARDARD_MAX_BITRATE 400000
> +#define FAST_MAX_BITRATE 1000000
> +#define FAST_PLUS_MAX_BITRATE 3400000
> +#define HIGHSPEED_MAX_BITRATE 5000000
> +
> +
> +enum lpi2c_imx_mode {
> + STANDARD, /* 100+Kbps */
> + FAST, /* 400+Kbps */
> + FAST_PLUS, /* 1.0+Mbps */
> + ULTRA_FAST, /* 5.0+Mbps */
> + HS, /* 3.4+Mbps */
> +};
> +
> +enum lpi2c_imx_pincfg {
> + TWO_PIN_OD, /* 2-pin open drain mode */
> + TWO_PIN_OO, /* 2-pin output only mode (utra-fast mode) */
> + TWO_PIN_PP, /* 2-pin push-pull mode */
> + FOUR_PIN_PP, /* 4-pin push-pull mode */
> + TWO_PIN_OD_SS, /* 2-pin open drain mode with separate slave */
> + TWO_PIN_OO_SS, /* 2-pin output only mode with separate slave */
> + TWO_PIN_PP_SS, /* 2-pin push-pull mode with separate slave */
> + FOUR_PIN_PP_IO, /* 4-pin push-pull mode (inverted output) */
> +};
> +
> +struct lpi2c_imx_clkcfg {
> + u8 prescale;
> + u8 filtscl;
> + u8 filtsda;
> + u8 sethold;
> + u8 clklo;
> + u8 clkhi;
> + u8 datavd;
> +};
> +
> +struct lpi2c_imx_struct {
> + struct i2c_adapter adapter;
> + struct clk *per_clk;
> + void __iomem *base;
> + __u8 *rx_buf;
> + __u8 *tx_buf;
> + struct completion complete;
> + unsigned int msglen;
> + unsigned int delivered;
> + unsigned int block_data;
> + unsigned int bitrate;
> + enum lpi2c_imx_mode mode;
> +};
> +
> +static void lpi2c_imx_intctrl(
> + struct lpi2c_imx_struct *lpi2c_imx, unsigned int enable) {
> + writel(enable, lpi2c_imx->base + LPI2C_MIER); }
> +
> +static int lpi2c_imx_bus_busy(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned long orig_jiffies = jiffies;
> + unsigned int temp;
> +
> + while (1) {
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> +
> + /* check for arbitration lost, clear if set */
> + if (temp & MSR_ALF) {
> + writel(temp, lpi2c_imx->base + LPI2C_MSR);
> + return -EAGAIN;
> + }
> +
> + if ((temp & MSR_BBF) && (temp & MSR_MBF))
> + break;
> +
> + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "bus not work\n");
> + return -ETIMEDOUT;
> + }
> + schedule();
> + }
> +
> + return 0;
> +}
> +
> +static void lpi2c_imx_set_mode(struct lpi2c_imx_struct *lpi2c_imx) {
> + enum lpi2c_imx_mode mode;
> + unsigned int bitrate = lpi2c_imx->bitrate;
> +
> + if (bitrate < STARDARD_MAX_BITRATE)
> + mode = STANDARD;
> + else if (bitrate < FAST_MAX_BITRATE)
> + mode = FAST;
> + else if (bitrate < FAST_PLUS_MAX_BITRATE)
> + mode = FAST_PLUS;
> + else if (bitrate < HIGHSPEED_MAX_BITRATE)
> + mode = HS;
> + else
> + mode = ULTRA_FAST;
> +
> + lpi2c_imx->mode = mode;
> +}
> +
> +static int lpi2c_imx_start(struct lpi2c_imx_struct *lpi2c_imx,
> + struct i2c_msg *msgs)
> +{
> + u8 read;
> + unsigned int temp;
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> + temp |= MCR_RRF | MCR_RTF;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> + writel(0x7f00, lpi2c_imx->base + LPI2C_MSR);
> +
> + read = msgs->flags & I2C_M_RD;
> + temp = (msgs->addr << 1 | read) | (GEN_START << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> +
> + return lpi2c_imx_bus_busy(lpi2c_imx);
> +}
> +
> +static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned int temp;
> + unsigned long orig_jiffies = jiffies;
> +
> + writel(GEN_STOP << 8, lpi2c_imx->base + LPI2C_MTDR);
> + do {
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> + if (temp & MSR_SDF)
> + break;
> +
> + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "stop timeout\n");
> + break;
> + }
> + schedule();
> +
> + } while (1);
> +}
> +
> +
> +/* CLKLO = I2C_CLK_RATIO * CLKHI, SETHOLD = CLKHI, DATAVD = CLKHI/2 */
> +static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned int temp;
> + unsigned int per_clk_rate;
> + unsigned int prescale, clk_high, clk_low, clk_cycle;
> + enum lpi2c_imx_pincfg pincfg;
> + struct lpi2c_imx_clkcfg clkcfg;
> +
> + lpi2c_imx_set_mode(lpi2c_imx);
> + per_clk_rate = clk_get_rate(lpi2c_imx->per_clk);
> +
> + if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
> + clkcfg.filtscl = clkcfg.filtsda = 0;
> + else
> + clkcfg.filtscl = clkcfg.filtsda = 2;
> +
> + for (prescale = 0; prescale <= 7; prescale++) {
> + clk_cycle = per_clk_rate / ((1 << prescale) * lpi2c_imx->bitrate)
> + - 3 - (clkcfg.filtscl >> 1);
> + clk_high = (clk_cycle + I2C_CLK_RATIO) / (I2C_CLK_RATIO + 1);
> + clk_low = clk_cycle - clk_high;
> + if (clk_low < 64)
> + break;
> + }
> +
> + if (prescale > 7)
> + return -EINVAL;
> +
> + clkcfg.prescale = prescale;
> + clkcfg.sethold = clk_high;
> + clkcfg.clklo = clk_low;
> + clkcfg.clkhi = clk_high;
> + clkcfg.datavd = clk_high >> 1;
> +
> + /* set MCFGR1: PINCFG, PRESCALE, IGNACK */
> + if (lpi2c_imx->mode == ULTRA_FAST)
> + pincfg = TWO_PIN_OO;
> + else
> + pincfg = TWO_PIN_OD;
> + temp = clkcfg.prescale | pincfg << 24;
> +
> + if (lpi2c_imx->mode == ULTRA_FAST)
> + temp |= MCFGR1_IGNACK;
> +
> + writel(temp, lpi2c_imx->base + LPI2C_MCFGR1);
> +
> + /* set MCFGR2: FILTSDA, FILTSCL */
> + temp = (clkcfg.filtscl << 16) | (clkcfg.filtsda << 24);
> + writel(temp, lpi2c_imx->base + LPI2C_MCFGR2);
> +
> +
> + /* set MCCR: DATAVD, SETHOLD, CLKHI, CLKLO */
> + temp = clkcfg.datavd << 24 | clkcfg.sethold << 16 |
> + clkcfg.clkhi << 8 | clkcfg.clklo;
> +
> + if (lpi2c_imx->mode == HS)
> + writel(temp, lpi2c_imx->base + LPI2C_MCCR1);
> + else
> + writel(temp, lpi2c_imx->base + LPI2C_MCCR0);
> +
> + return 0;
> +}
> +
> +static int lpi2c_imx_master_enable(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + int ret;
> + unsigned int temp;
> +
> + ret = clk_prepare_enable(lpi2c_imx->per_clk);
> + if (ret)
> + return ret;
> +
> + temp = MCR_RST;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> + writel(0, lpi2c_imx->base + LPI2C_MCR);
> +
> + ret = lpi2c_imx_config(lpi2c_imx);
> + if (ret)
> + return ret;
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> + temp |= MCR_MEN;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> +
> + return 0;
> +}
> +
> +static int lpi2c_imx_master_disable(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int temp = 0;
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> + temp &= ~MCR_MEN;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> +
> + clk_disable_unprepare(lpi2c_imx->per_clk);
> +
> + return 0;
> +}
> +
> +static int lpi2c_imx_msg_complete(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned int timeout;
> +
> + timeout = wait_for_completion_timeout(&lpi2c_imx->complete, HZ);
> +
> + return timeout ? 0 : -ETIMEDOUT;
> +}
> +
> +static int lpi2c_imx_txfifo_empty(struct lpi2c_imx_struct *lpi2c_imx) {
> + u32 txcnt;
> + unsigned long orig_jiffies = jiffies;
> +
> + do {
> + txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
> +
> + if (readl(lpi2c_imx->base + LPI2C_MSR) & MSR_NDF) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "NDF detected\n");
> + return -EIO;
> + }
> +
> + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "txfifo empty
> timeout\n");
> + return -ETIMEDOUT;
> + }
> + schedule();
> +
> + } while (txcnt);
> +
> + return 0;
> +}
> +
> +static void lpi2c_imx_set_tx_watermark(struct lpi2c_imx_struct
> +*lpi2c_imx) {
> + unsigned int temp;
> +
> + temp = LPI2C_TX_FIFOSIZE >> 1;
> + writel(temp, lpi2c_imx->base + LPI2C_MFCR); }
> +
> +static void lpi2c_imx_set_rx_watermark(struct lpi2c_imx_struct
> +*lpi2c_imx) {
> + unsigned int temp, remaining;
> +
> + remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
> +
> + if (remaining > (LPI2C_RX_FIFOSIZE >> 1))
> + temp = LPI2C_RX_FIFOSIZE >> 1;
> + else
> + temp = 0;
> +
> + writel(temp << 16, lpi2c_imx->base + LPI2C_MFCR); }
> +
> +static void lpi2c_imx_write_txfifo(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int data, txcnt;
> +
> + txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
> + while (txcnt < LPI2C_TX_FIFOSIZE) {
> + if (lpi2c_imx->delivered == lpi2c_imx->msglen)
> + break;
> + data = lpi2c_imx->tx_buf[lpi2c_imx->delivered++];
> + writel(data, lpi2c_imx->base + LPI2C_MTDR);
> + txcnt++;
> + }
> +
> + if (lpi2c_imx->delivered < lpi2c_imx->msglen)
> + lpi2c_imx_intctrl(lpi2c_imx, MIER_TDIE | MIER_NDIE);
> + else
> + complete(&lpi2c_imx->complete);
> +}
> +
> +static void lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned int temp, data;
> + unsigned int blocklen, remaining;
> +
> + do {
> + data = readl(lpi2c_imx->base + LPI2C_MRDR);
> + if (data & MRDR_RXEMPTY)
> + break;
> + lpi2c_imx->rx_buf[lpi2c_imx->delivered++] = data & 0xff;
> + } while (1);
> +
> + /*
> + * First byte is the length of remaining packet in the SMBus block
> + * data read. Add it to msgs->len.
> + */
> + if (lpi2c_imx->block_data) {
> + blocklen = lpi2c_imx->rx_buf[0];
> + lpi2c_imx->msglen += blocklen;
> + }
> +
> + remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
> + /* not finished, still waiting for rx data */
> + if (remaining) {
> + lpi2c_imx_set_rx_watermark(lpi2c_imx);
> + /* multiple receive commands */
> + if (lpi2c_imx->block_data) {
> + lpi2c_imx->block_data = 0;
> + temp = remaining;
> + temp |= (RECV_DATA << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> + } else if (!(lpi2c_imx->delivered & 0xff)) {
> + temp = remaining > CHUNK_DATA ?
> + CHUNK_DATA - 1 : (remaining - 1);
> + temp |= (RECV_DATA << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> + }
> +
> + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE);
> + } else
> + complete(&lpi2c_imx->complete);
> +}
> +
> +static void lpi2c_imx_write(struct lpi2c_imx_struct *lpi2c_imx,
> + struct i2c_msg *msgs)
> +{
> + lpi2c_imx->tx_buf = msgs->buf;
> + lpi2c_imx_set_tx_watermark(lpi2c_imx);
> + lpi2c_imx_write_txfifo(lpi2c_imx);
> +}
> +
> +static void lpi2c_imx_read(struct lpi2c_imx_struct *lpi2c_imx,
> + struct i2c_msg *msgs)
> +{
> + unsigned int temp;
> +
> + lpi2c_imx->rx_buf = msgs->buf;
> + lpi2c_imx->block_data = msgs->flags & I2C_M_RECV_LEN;
> +
> + lpi2c_imx_set_rx_watermark(lpi2c_imx);
> + temp = msgs->len > CHUNK_DATA ? CHUNK_DATA - 1 : msgs->len - 1;
> + temp |= (RECV_DATA << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> +
> + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE | MIER_NDIE); }
> +
> +static int lpi2c_imx_xfer(struct i2c_adapter *adapter,
> + struct i2c_msg *msgs, int num)
> +{
> + int i, result;
> + unsigned int temp;
> + struct lpi2c_imx_struct *lpi2c_imx = i2c_get_adapdata(adapter);
> +
> + result = lpi2c_imx_master_enable(lpi2c_imx);
> + if (result)
> + return result;
> +
> + for (i = 0; i < num; i++) {
> + result = lpi2c_imx_start(lpi2c_imx, &msgs[i]);
> + if (result)
> + goto disable;
> +
> + /* quick smbus */
> + if (num == 1 && msgs[0].len == 0)
> + goto stop;
> +
> + lpi2c_imx->delivered = 0;
> + lpi2c_imx->msglen = msgs[i].len;
> + init_completion(&lpi2c_imx->complete);
> +
> + if (msgs[i].flags & I2C_M_RD)
> + lpi2c_imx_read(lpi2c_imx, &msgs[i]);
> + else
> + lpi2c_imx_write(lpi2c_imx, &msgs[i]);
> +
> + result = lpi2c_imx_msg_complete(lpi2c_imx);
> + if (result)
> + goto stop;
> +
> + if (!(msgs[i].flags & I2C_M_RD)) {
> + result = lpi2c_imx_txfifo_empty(lpi2c_imx);
> + if (result)
> + goto stop;
> + }
> + }
> +
> +stop:
> + lpi2c_imx_stop(lpi2c_imx);
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> + if ((temp & MSR_NDF) && !result)
> + result = -EIO;
> +
> +disable:
> + lpi2c_imx_master_disable(lpi2c_imx);
> +
> + dev_dbg(&lpi2c_imx->adapter.dev, "<%s> exit with: %s: %d\n", __func__,
> + (result < 0) ? "error" : "success msg",
> + (result < 0) ? result : num);
> +
> + return (result < 0) ? result : num;
> +}
> +
> +static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id) {
> + unsigned int temp;
> + struct lpi2c_imx_struct *lpi2c_imx = dev_id;
> +
> + lpi2c_imx_intctrl(lpi2c_imx, 0);
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> +
> + if (temp & MSR_RDF) {
> + lpi2c_imx_read_rxfifo(lpi2c_imx);
> + return IRQ_HANDLED;
> + }
> +
> + if (temp & MSR_TDF) {
> + lpi2c_imx_write_txfifo(lpi2c_imx);
> + return IRQ_HANDLED;
> + }
> +
> + complete(&lpi2c_imx->complete);
> + return IRQ_HANDLED;
> +}
> +
> +static u32 lpi2c_imx_func(struct i2c_adapter *adapter) {
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
> + | I2C_FUNC_SMBUS_READ_BLOCK_DATA;
> +}
> +
> +static struct i2c_algorithm lpi2c_imx_algo = {
> + .master_xfer = lpi2c_imx_xfer,
> + .functionality = lpi2c_imx_func,
> +};
> +
> +static const struct of_device_id lpi2c_imx_of_match[] = {
> + { .compatible = "fsl,imx8dv-lpi2c" },
> + { .compatible = "fsl,imx7ulp-lpi2c" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, lpi2c_imx_of_match)
> +
> +static int lpi2c_imx_probe(struct platform_device *pdev) {
> + int irq, ret;
> + void __iomem *base;
> + struct resource *res;
> + struct lpi2c_imx_struct *lpi2c_imx;
> +
> + lpi2c_imx = devm_kzalloc(&pdev->dev,
> + sizeof(*lpi2c_imx), GFP_KERNEL);
> + if (!lpi2c_imx)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "can't get irq number\n");
> + return irq;
> + }
> +
> + lpi2c_imx->adapter.owner = THIS_MODULE;
> + lpi2c_imx->adapter.algo = &lpi2c_imx_algo;
> + lpi2c_imx->adapter.dev.parent = &pdev->dev;
> + lpi2c_imx->adapter.nr = pdev->id;
> + lpi2c_imx->base = base;
> + lpi2c_imx->adapter.dev.of_node = pdev->dev.of_node;
> + strlcpy(lpi2c_imx->adapter.name, pdev->name,
> + sizeof(lpi2c_imx->adapter.name));
> +
> + lpi2c_imx->per_clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(lpi2c_imx->per_clk)) {
> + dev_err(&pdev->dev, "can't get I2C peripheral clock\n");
> + return PTR_ERR(lpi2c_imx->per_clk);
> + }
> +
> + ret = of_property_read_u32(pdev->dev.of_node,
> + "clock-frequency", &lpi2c_imx->bitrate);
> + if (ret)
> + lpi2c_imx->bitrate = LPI2C_DEFAULT_RATE;
> +
> + ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, 0,
> + pdev->name, lpi2c_imx);
> + if (ret) {
> + dev_err(&pdev->dev, "can't claim irq %d\n", irq);
> + goto ret;
> + }
> +
> + i2c_set_adapdata(&lpi2c_imx->adapter, lpi2c_imx);
> + platform_set_drvdata(pdev, lpi2c_imx);
> +
> + ret = i2c_add_numbered_adapter(&lpi2c_imx->adapter);
> + if (ret) {
> + dev_err(&pdev->dev, "registration failed\n");
> + goto ret;
> + }
> +
> + dev_info(&lpi2c_imx->adapter.dev, "LPI2C adapter registered\n");
> +
> +ret:
> + return ret;
> +}
> +
> +static int lpi2c_imx_remove(struct platform_device *pdev) {
> + struct lpi2c_imx_struct *lpi2c_imx = platform_get_drvdata(pdev);
> +
> + i2c_del_adapter(&lpi2c_imx->adapter);
> +
> + return 0;
> +}
> +
> +static struct platform_driver lpi2c_imx_driver = {
> + .probe = lpi2c_imx_probe,
> + .remove = lpi2c_imx_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .of_match_table = lpi2c_imx_of_match,
> + },
> +};
> +
> +static int __init i2c_adap_imx_init(void) {
> + return platform_driver_register(&lpi2c_imx_driver);
> +}
> +module_init(i2c_adap_imx_init);
> +
> +static void __exit i2c_adap_imx_exit(void) {
> + platform_driver_unregister(&lpi2c_imx_driver);
> +}
> +module_exit(i2c_adap_imx_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Gao Pan <pandy.gao@nxp.com>");
> MODULE_DESCRIPTION("I2C
> +adapter driver for LPI2C bus"); MODULE_ALIAS("platform:" DRIVER_NAME);
> --
> 1.9.1
^ permalink raw reply
* Re: [RFC 00/17] clk: Add per-controller locks to fix deadlocks
From: Stephen Boyd @ 2016-09-09 0:24 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Michael Turquette, Stephen Warren, Lee Jones, Eric Anholt,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Sylwester Nawrocki, Tomasz Figa,
Kukjin Kim, Russell King, Mark Brown, linux-clk, linux-rpi-kernel,
linux-arm-kernel, linux-kernel, linux-samsung-soc, linux-i2c,
alsa-devel, Marek Szyprowski, Charles
In-Reply-To: <1471354514-24224-1-git-send-email-k.kozlowski@samsung.com>
On 08/16, Krzysztof Kozlowski wrote:
> Hi,
>
> RFC, please, do not apply, maybe except patch #1 which is harmless.
>
>
> Introduction
> ============
> The patchset brings new entity: clock controller representing a hardware
> block. The clock controller comes with its own prepare lock which
> is used then in many places. The idea is to fix the deadlock mentioned
> in commit 10ff4c5239a1 ("i2c: exynos5: Fix possible ABBA deadlock by keeping
> I2C clock prepared") and commit 34e81ad5f0b6 ("i2c: s3c2410: fix ABBA deadlock
> by keeping clock prepared").
>
>
> Disclaimer
> ==========
> Request for comments, so:
> 1. Only exynos_defconfig builds,
> 2. A lot of FIXME/TODO note still,
> 3. Checkpatch not run, lines not aligned,
> 4. Other (non-exynos) drivers not converted,
> 5. Probably not yet bisectable,
> 6. Locking became quite complex.
> The previous one lock was simple. Inefficient and dead-lock prone but
> simple. Because of clock hierarchy spanning through controllers, the
> new locking became quite complicated. I don't like it but...
>
>
> Details
> =======
> In Exynos-based boards case the deadlock occurs between clock's
> prepare_lock and regmap-i2c's lock:
>
> CPU #0: CPU #1:
> lock(regmap)
> s2mps11-clk: clk_prepare_lock()
>
> i2c-exynos: clk_prepare_lock() - wait
> lock(regmap) - wait
>
> The clk_prepare_lock() on both CPUs come from different clock drivers
> and components:
> 1. I2C clock is part of SoC block and is required by i2c-s3c2410/i2c-exynos5
> driver,
> 2. S2MPS11 clock is separate device, however driver uses I2C regmap.
>
> The deadlock was reported by lockdep (always) and was happening
> in 20% of boots of Odroid XU3 with multi_v7 defconfig. Workaround for
> deadlock was implemented by removing prepare/unprepare calls from I2C
> transfers. However these are just workarounds... which after applying
> this patch can be reverted.
>
> Additionally Marek Szyprowski's work on domains/clocks/pinctrl exposed
> the deadlock again in different configuration.
>
Per-controller locks seems to be a misnomer. These patches look
more like per-clk_register() locks. The clk registrant can decide
how fine grained to make the prepare locking scheme. They can
decide to have one lock per clk, one lock for the entire tree, or
one lock per arbitrary subtree. I'd prefer to drop the controller
concept unless it's really done that way, by attaching to the OF
clk provider or device pointer.
Given that, it seems that conceptually these patches allow clk
registrants to carve out a set of clks that they want to be
treated as an atomic unit with regards to the prepare lock.
TL;DR trees of per-process recursive prepare mutex locks where
the locks cover one or more clks.
The locks are recursive per-process to simplify the problem where
we need to acquire the same lock multiple times while traversing
a tree that shares the same lock, right? Otherwise we would need
to track which locks we've already grabbed and refcount and we
wouldn't be able to re-enter the framework from within the clk
ops of the same atomic unit.
As long as we can guarantee that we always take the locks in the
same order we'll be fine. The burden to ensure that would be
placed on the clk registrants though, and verifying that will be
left up to humans? That seems fragile and error prone. We can
always say that we take the global lock and then traverse the
children up to the roots, but we can't be sure that during the
tree traversal we don't take locks in different order because of
how the clk registrant has structured things.
It would be great if the different clk APIs were listed, with the
order of how locks are taken BTW. And really overall just more
information about how the locking scheme works. I had to read
this patch series quite a few times to come to (hopefully the
right) conclusions.
So I'm not very fond of this design because the locking scheme is
pretty much out of the hands of the framework and can be easily
broken. I'm biased of course, because I'd prefer we go with my
wwmutex design of per-clk locks[1]. Taking locks in any order
works fine there, and we resolve quite a few long standing
locking problems that we have while improving scalability. The
problem there is that we don't get the recursive mutex design
(maybe that's a benefit!). Once a clk_op reenters the framework
with consumer APIs and tries to grab the same lock we deadlock.
This is why I've been slowly splitting consumers from providers
so we can easily identify these cases. If we had something like
coordinated clk rate switching, we could get rid of clk_ops
reentering the framework and avoid this problem (and we really do
need to do that).
The one thing I do like about this patch series though is the
gradual movement of providers to the new locking scheme. Maybe if
we combined that idea with per-clk wwmutex locks we could have
the best of both worlds and fix reentrancy later when we need it?
There's wwmutex_trylock() that we could use. Perhaps always pass
a NULL context if some clk flag isn't set (CLK_USE_PER_CLK_LOCK).
Plus we could assign the same wwmutex all over the place when
that flag isn't set and allocate a unique wwmutex at clk_register
time otherwise. I haven't thought about it deeply, so there may
be some glaring problem that I'm missing like when we have half
the clks in a tree with the flag set or something. This is where
driving home from the office for 45 minutes helps.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/251039.html
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v5] i2c: imx: make bus recovery through pinctrl optional
From: Leo Li @ 2016-09-08 23:57 UTC (permalink / raw)
To: Stefan Agner
Cc: Uwe Kleine-König, Gao Pan, Wolfram Sang, lkml, Li Yang,
linux-gpio, linux-i2c@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <8f8f7ffc4aef513ef5064ba0491fbc63@agner.ch>
On Thu, Sep 8, 2016 at 5:39 PM, Stefan Agner <stefan@agner.ch> wrote:
> On 2016-09-06 15:40, Leo Li wrote:
>> On Tue, Sep 6, 2016 at 4:51 PM, Stefan Agner <stefan@agner.ch> wrote:
>>> On 2016-09-06 13:06, Leo Li wrote:
>>>> On Tue, Sep 6, 2016 at 1:58 PM, Uwe Kleine-König
>>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>>> On Fri, Aug 19, 2016 at 05:05:22PM -0500, Li Yang wrote:
> <snip>
>>>>>> @@ -1081,8 +1090,11 @@ static int i2c_imx_probe(struct platform_device *pdev)
>>>>>> return ret;
>>>>>> }
>>>>>>
>>>>>> + /* optional bus recovery feature through pinctrl */
>>>>>> i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
>>>>>> - if (IS_ERR(i2c_imx->pinctrl)) {
>>>>>> + /* bailout on -ENOMEM or -EPROBE_DEFER, continue for other errors */
>>>>>> + if (PTR_ERR(i2c_imx->pinctrl) == -ENOMEM ||
>>>>>> + PTR_ERR(i2c_imx->pinctrl) == -EPROBE_DEFER) {
>>>>>> ret = PTR_ERR(i2c_imx->pinctrl);
>>>>>> goto clk_disable;
>>>>>> }
>>>>>
>>>>> devm_pinctrl_get might return the following error-valued pointers:
>>>>> - -EINVAL
>>>>> - -ENOMEM
>>>>> - -ENODEV
>>>>> - -EPROBE_DEFER
>>>>>
>>>>> There are several error paths returning -EINVAL, one is when an invalid
>>>>> phandle is used. Do you really want to ignore that?
>>>>>
>>>>> IMO error handling is better done with inverse logic, that is continue
>>>>> on some explicit error, bail out on all unknown stuff. This tends to be
>>>>> more robust. Also the comment should be improved to not explain that for
>>>>> -ENOMEM and -EPROBE_DEFER we bail out (which should be obvious for
>>>>> anyone who can read C) but to explain why.
>>>>
>>>> What you said is true for normal error handling, but in this scenario
>>>> it is intentional to ignore all pinctrl related errors except critical
>>>> ones because failing to have pinctrl for an optional feature shouldn't
>>>> impact the function of normal i2c. We choose to catch -ENOMEM because
>>>> the error could also cause problem for i2c probe, and -EPROBE_DEFER
>>>> because it's possible that the pinctrl will be ready later and we want
>>>> to give it a chance. The i2c driver really don't care why the pinctrl
>>>> was not usable. I thought I added comment before the
>>>
>>> I don't agree. E.g. -EINVAL would appear if you pass devm_pinctrl_get an
>>> invalid device. Currently you would silently ignore that, which is not
>>> what you want.
>>
>> It is not silently ignored, there will be a message printed out saying
>> pinctrl is not available and bus recovery is not supported. On the
>> contrary, without this change the entire i2c driver fails to work
>> silently if pinctrl is somehow not working. And if the system is so
>> broken that the pointer to the i2c device is NULL, the probe of i2c
>> would have already failed before this point. We shouldn't count on an
>> optional function of the driver to catch fundamental issues like this.
>>
>>>
>>> You want to get the pinctrl in any case expect there isn't one. And that
>>> is how you should formulate your if statement.
>>>
>>> /*
>>> * It is ok if no pinctrl device is available. We'll not be able to use
>>> the
>>> * bus recovery feature, but otherwise the driver works fine...
>>> */
>>> if (PTR_ERR(i2c_imx->pinctrl) != -ENODEV)
>>
>> I agree that there could be other possibilities that the pinctrl
>> failed to work beside the reason I described in the commit
>> message(platform doesn't support pinctrl at all). But I don't think
>> any of them other than the -ENOMEM and -EPROBE_DEFER deserves a bail
>> out for the entire i2c driver.
>
> FWIW, I disagree. If there is pinctrl defined, you want be sure that it
> gets applied properly, no matter what. E.g. when devm_pinctrl_get return
> EINVAL (Uwe's example) the driver will continue and likely fail in
> mysterious ways later on because the pins have not been muxed properly.
> The driver should not load in that situation so that the developer is
> forced to fix his mistakes. The only reason to bail out here is if there
> is no pin controller (ENODEV). And it seems that Uwe also tends to that
> solution.
With this patch the i2c bus recovery feature will be disabled if the
devm_pinctrl_get() fails. The pin mux setting will not be changed in
either i2c probe stage or at runtime. I don't think it can cause any
trouble to normal I2C operation. IMO, it is not good to *force*
people fix problem that they don't really care by deliberately enlarge
the problem. That's why we don't panic() on any error we found. For
those who do care about the bus recovery, they can get the information
from the console.
Regards,
Leo
^ permalink raw reply
* Re: [PATCH v5] i2c: imx: make bus recovery through pinctrl optional
From: Stefan Agner @ 2016-09-08 22:39 UTC (permalink / raw)
To: Leo Li
Cc: Uwe Kleine-König, Gao Pan, Wolfram Sang, lkml, Li Yang,
linux-gpio, linux-i2c, linux-arm-kernel
In-Reply-To: <CADRPPNSPye45bmFshfwb+gDA0x0ygs4Ofa6HMYbKt9ER0oeccg@mail.gmail.com>
On 2016-09-06 15:40, Leo Li wrote:
> On Tue, Sep 6, 2016 at 4:51 PM, Stefan Agner <stefan@agner.ch> wrote:
>> On 2016-09-06 13:06, Leo Li wrote:
>>> On Tue, Sep 6, 2016 at 1:58 PM, Uwe Kleine-König
>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>> On Fri, Aug 19, 2016 at 05:05:22PM -0500, Li Yang wrote:
<snip>
>>>>> @@ -1081,8 +1090,11 @@ static int i2c_imx_probe(struct platform_device *pdev)
>>>>> return ret;
>>>>> }
>>>>>
>>>>> + /* optional bus recovery feature through pinctrl */
>>>>> i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
>>>>> - if (IS_ERR(i2c_imx->pinctrl)) {
>>>>> + /* bailout on -ENOMEM or -EPROBE_DEFER, continue for other errors */
>>>>> + if (PTR_ERR(i2c_imx->pinctrl) == -ENOMEM ||
>>>>> + PTR_ERR(i2c_imx->pinctrl) == -EPROBE_DEFER) {
>>>>> ret = PTR_ERR(i2c_imx->pinctrl);
>>>>> goto clk_disable;
>>>>> }
>>>>
>>>> devm_pinctrl_get might return the following error-valued pointers:
>>>> - -EINVAL
>>>> - -ENOMEM
>>>> - -ENODEV
>>>> - -EPROBE_DEFER
>>>>
>>>> There are several error paths returning -EINVAL, one is when an invalid
>>>> phandle is used. Do you really want to ignore that?
>>>>
>>>> IMO error handling is better done with inverse logic, that is continue
>>>> on some explicit error, bail out on all unknown stuff. This tends to be
>>>> more robust. Also the comment should be improved to not explain that for
>>>> -ENOMEM and -EPROBE_DEFER we bail out (which should be obvious for
>>>> anyone who can read C) but to explain why.
>>>
>>> What you said is true for normal error handling, but in this scenario
>>> it is intentional to ignore all pinctrl related errors except critical
>>> ones because failing to have pinctrl for an optional feature shouldn't
>>> impact the function of normal i2c. We choose to catch -ENOMEM because
>>> the error could also cause problem for i2c probe, and -EPROBE_DEFER
>>> because it's possible that the pinctrl will be ready later and we want
>>> to give it a chance. The i2c driver really don't care why the pinctrl
>>> was not usable. I thought I added comment before the
>>
>> I don't agree. E.g. -EINVAL would appear if you pass devm_pinctrl_get an
>> invalid device. Currently you would silently ignore that, which is not
>> what you want.
>
> It is not silently ignored, there will be a message printed out saying
> pinctrl is not available and bus recovery is not supported. On the
> contrary, without this change the entire i2c driver fails to work
> silently if pinctrl is somehow not working. And if the system is so
> broken that the pointer to the i2c device is NULL, the probe of i2c
> would have already failed before this point. We shouldn't count on an
> optional function of the driver to catch fundamental issues like this.
>
>>
>> You want to get the pinctrl in any case expect there isn't one. And that
>> is how you should formulate your if statement.
>>
>> /*
>> * It is ok if no pinctrl device is available. We'll not be able to use
>> the
>> * bus recovery feature, but otherwise the driver works fine...
>> */
>> if (PTR_ERR(i2c_imx->pinctrl) != -ENODEV)
>
> I agree that there could be other possibilities that the pinctrl
> failed to work beside the reason I described in the commit
> message(platform doesn't support pinctrl at all). But I don't think
> any of them other than the -ENOMEM and -EPROBE_DEFER deserves a bail
> out for the entire i2c driver.
FWIW, I disagree. If there is pinctrl defined, you want be sure that it
gets applied properly, no matter what. E.g. when devm_pinctrl_get return
EINVAL (Uwe's example) the driver will continue and likely fail in
mysterious ways later on because the pins have not been muxed properly.
The driver should not load in that situation so that the developer is
forced to fix his mistakes. The only reason to bail out here is if there
is no pin controller (ENODEV). And it seems that Uwe also tends to that
solution.
--
Stefan
^ permalink raw reply
* Re: [PATCH] i2c: rk3x: Restore clock settings at resume time
From: Wolfram Sang @ 2016-09-08 20:50 UTC (permalink / raw)
To: Douglas Anderson
Cc: heiko, linux-rockchip, david.wu, wxt, linux-arm-kernel, linux-i2c,
linux-kernel
In-Reply-To: <1472505756-8302-1-git-send-email-dianders@chromium.org>
[-- Attachment #1: Type: text/plain, Size: 1702 bytes --]
On Mon, Aug 29, 2016 at 02:22:36PM -0700, Douglas Anderson wrote:
> Depending on a number of factors including:
> - Which exact Rockchip SoC we're working with
> - How deep we suspend
> - Which i2c port we're on
>
> We might lose the state of the i2c registers at suspend time.
> Specifically we've found that on rk3399 the i2c ports that are not in
> the PMU power domain lose their state with the current suspend depth
> configured by ARM Tursted Firmware.
>
> Note that there are very few actual i2c registers that aren't configured
> per transfer anyway so all we actually need to re-configure are the
> clock config registers. We'll just add a call to rk3x_i2c_adapt_div()
> at resume time and be done with it.
>
> NOTE: On rk3399 on ports whose power was lost, I put printouts in at
> resume time. I saw things like:
> before: con=0x00010300, div=0x00060006
> after: con=0x00010200, div=0x00180025
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Fixed the code checker warnings:
SPARSE
drivers/i2c/busses/i2c-rk3x.c:1346:14: warning: duplicate const
SMATCH
drivers/i2c/busses/i2c-rk3x.c:1346:14: warning: duplicate const
and applied to for-current, thanks!
Note my code checkers also say this (unrelated to your patch):
SPARSE
drivers/i2c/busses/i2c-rk3x.c:888:17: warning: cast truncates bits from constant value (ffffffffff00 becomes ffffff00)
CC drivers/i2c/busses/i2c-rk3x.o
drivers/i2c/busses/i2c-rk3x.c: In function 'rk3x_i2c_v1_calc_timings':
drivers/i2c/busses/i2c-rk3x.c:745:41: warning: variable 'min_total_ns' set but not used [-Wunused-but-set-variable]
I haven't checked those, though...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 0/3] i2c: uniphier: updates for v4.9
From: Wolfram Sang @ 2016-09-08 20:41 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: linux-i2c, linux-arm-kernel, linux-kernel
In-Reply-To: <1472730390-8907-1-git-send-email-yamada.masahiro@socionext.com>
[-- Attachment #1: Type: text/plain, Size: 346 bytes --]
On Thu, Sep 01, 2016 at 08:46:27PM +0900, Masahiro Yamada wrote:
>
>
>
> Masahiro Yamada (3):
> i2c: uniphier: avoid WARN_ON() of clk_disable() in failure path
> i2c: uniphier-f: avoid WARN_ON() of clk_disable() in failure path
> i2c: uniphier-f: set the adapter to master mode when probing
>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v11 1/5] i2c: tegra: use readl_poll_timeout after config_load reg programmed
From: Wolfram Sang @ 2016-09-08 20:37 UTC (permalink / raw)
To: Shardar Shariff Md
Cc: swarren-3lzwWm7+Weoh9ZMKESR00Q,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
gnurou-Re5JQEeQqe8AvxtiuMwx3w, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jonathanh-DDmLM1+adcrQT0dZR+AlfA,
ldewangan-DDmLM1+adcrQT0dZR+AlfA
In-Reply-To: <1472650124-5702-1-git-send-email-smohammed-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 386 bytes --]
On Wed, Aug 31, 2016 at 06:58:40PM +0530, Shardar Shariff Md wrote:
> After CONFIG_LOAD register is programmed instead of explicitly waiting
> for timeout, use readl_poll_timeout() to check for register value to get
> updated or wait till timeout.
>
> Signed-off-by: Shardar Shariff Md <smohammed-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Series applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] i2c: tegra: Fix assignment of boolean variables
From: Wolfram Sang @ 2016-09-08 20:26 UTC (permalink / raw)
To: Jon Hunter
Cc: Laxman Dewangan, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1473155445-3046-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 499 bytes --]
On Tue, Sep 06, 2016 at 10:50:45AM +0100, Jon Hunter wrote:
> Fix the following warnings reported by coccinelle for the Tegra I2C
> driver.
>
> drivers/i2c/busses/i2c-tegra.c:513:2-23: WARNING: Assignment of bool to 0/1
> drivers/i2c/busses/i2c-tegra.c:539:3-24: WARNING: Assignment of bool to 0/1
>
> Reported-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] i2c: shmobile: Use ARCH_SHMOBILE instead of SUPERH
From: Wolfram Sang @ 2016-09-08 20:24 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: linux-i2c, linux-sh, linux-renesas-soc
In-Reply-To: <1472636113-26988-1-git-send-email-geert+renesas@glider.be>
[-- Attachment #1: Type: text/plain, Size: 360 bytes --]
On Wed, Aug 31, 2016 at 11:35:13AM +0200, Geert Uytterhoeven wrote:
> "i2c-sh_mobile" is used on sh7343, sh7366, sh7722, sh7723, and sh7724
> only. As all of the above select ARCH_SHMOBILE, restrict its driver
> dependencies from SUPERH to ARCH_SHMOBILE.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] [trivial] i2c: Spelling s/acknowedge/acknowledge/
From: Wolfram Sang @ 2016-09-08 20:22 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Jiri Kosina, linux-i2c, linux-kernel
In-Reply-To: <1472636328-27229-1-git-send-email-geert+renesas@glider.be>
[-- Attachment #1: Type: text/plain, Size: 164 bytes --]
On Wed, Aug 31, 2016 at 11:38:48AM +0200, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Applied to for-current, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox