The arche driver currently depends on the out-of-tree usb3613 driver and this prevented a recent compile breakage after removing the timesync code from being detected.
These patches remove the remaining timesync bits, and enable compile testing of the arche driver to avoid further unintended breakage.
Johan
Johan Hovold (2): staging: greybus: arche: remove timesync remains staging: greybus: enable compile testing of arche driver
drivers/staging/greybus/Kconfig | 10 +++ drivers/staging/greybus/Makefile | 2 +- drivers/staging/greybus/arche-apb-ctrl.c | 11 +-- drivers/staging/greybus/arche-platform.c | 150 ++----------------------------- drivers/staging/greybus/arche_platform.h | 8 -- 5 files changed, 21 insertions(+), 160 deletions(-)
Remove the remaining timesync bits that were left in the arche platform driver and which prevented the driver from being compiled.
Fixes: bdfb95c4baab ("staging: greybus: remove timesync protocol support") Signed-off-by: Johan Hovold johan@kernel.org --- drivers/staging/greybus/arche-apb-ctrl.c | 11 +-- drivers/staging/greybus/arche-platform.c | 143 ------------------------------- drivers/staging/greybus/arche_platform.h | 8 -- 3 files changed, 3 insertions(+), 159 deletions(-)
diff --git a/drivers/staging/greybus/arche-apb-ctrl.c b/drivers/staging/greybus/arche-apb-ctrl.c index 02243b4fd898..0412f3d06efb 100644 --- a/drivers/staging/greybus/arche-apb-ctrl.c +++ b/drivers/staging/greybus/arche-apb-ctrl.c @@ -22,6 +22,8 @@ #include "arche_platform.h"
+static void apb_bootret_deassert(struct device *dev); + struct arche_apb_ctrl_drvdata { /* Control GPIO signals to and from AP <=> AP Bridges */ int resetn_gpio; @@ -222,14 +224,7 @@ static void poweroff_seq(struct platform_device *pdev) /* TODO: May have to send an event to SVC about this exit */ }
-void apb_bootret_assert(struct device *dev) -{ - struct arche_apb_ctrl_drvdata *apb = dev_get_drvdata(dev); - - gpio_set_value(apb->boot_ret_gpio, 1); -} - -void apb_bootret_deassert(struct device *dev) +static void apb_bootret_deassert(struct device *dev) { struct arche_apb_ctrl_drvdata *apb = dev_get_drvdata(dev);
diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c index aac1145f1983..9e644bfe2ae0 100644 --- a/drivers/staging/greybus/arche-platform.c +++ b/drivers/staging/greybus/arche-platform.c @@ -35,7 +35,6 @@ enum svc_wakedetect_state { WD_STATE_STANDBYBOOT_TRIG, /* As of now not used ?? */ WD_STATE_COLDBOOT_START, /* Cold boot process started */ WD_STATE_STANDBYBOOT_START, /* Not used */ - WD_STATE_TIMESYNC, };
struct arche_platform_drvdata { @@ -59,26 +58,12 @@ struct arche_platform_drvdata { int wake_detect_irq; spinlock_t wake_lock; /* Protect wake_detect_state */ struct mutex platform_state_mutex; /* Protect state */ - wait_queue_head_t wq; /* WQ for arche_pdata->state */ unsigned long wake_detect_start; struct notifier_block pm_notifier;
struct device *dev; - struct gb_timesync_svc *timesync_svc_pdata; };
-static int arche_apb_bootret_assert(struct device *dev, void *data) -{ - apb_bootret_assert(dev); - return 0; -} - -static int arche_apb_bootret_deassert(struct device *dev, void *data) -{ - apb_bootret_deassert(dev); - return 0; -} - /* Requires calling context to hold arche_pdata->platform_state_mutex */ static void arche_platform_set_state(struct arche_platform_drvdata *arche_pdata, enum arche_platform_state state) @@ -86,112 +71,6 @@ static void arche_platform_set_state(struct arche_platform_drvdata *arche_pdata, arche_pdata->state = state; }
-/* - * arche_platform_change_state: Change the operational state - * - * This exported function allows external drivers to change the state - * of the arche-platform driver. - * Note that this function only supports transitions between two states - * with limited functionality. - * - * - ARCHE_PLATFORM_STATE_TIME_SYNC: - * Once set, allows timesync operations between SVC <=> AP and makes - * sure that arche-platform driver ignores any subsequent events/pulses - * from SVC over wake/detect. - * - * - ARCHE_PLATFORM_STATE_ACTIVE: - * Puts back driver to active state, where any pulse from SVC on wake/detect - * line would trigger either cold/standby boot. - * Note: Transition request from this function does not trigger cold/standby - * boot. It just puts back driver book keeping variable back to ACTIVE - * state and restores the interrupt. - * - * Returns -ENODEV if device not found, -EAGAIN if the driver cannot currently - * satisfy the requested state-transition or -EINVAL for all other - * state-transition requests. - */ -int arche_platform_change_state(enum arche_platform_state state, - struct gb_timesync_svc *timesync_svc_pdata) -{ - struct arche_platform_drvdata *arche_pdata; - struct platform_device *pdev; - struct device_node *np; - int ret = -EAGAIN; - unsigned long flags; - - np = of_find_compatible_node(NULL, NULL, "google,arche-platform"); - if (!np) { - pr_err("google,arche-platform device node not found\n"); - return -ENODEV; - } - - pdev = of_find_device_by_node(np); - if (!pdev) { - pr_err("arche-platform device not found\n"); - of_node_put(np); - return -ENODEV; - } - - arche_pdata = platform_get_drvdata(pdev); - - mutex_lock(&arche_pdata->platform_state_mutex); - spin_lock_irqsave(&arche_pdata->wake_lock, flags); - - if (arche_pdata->state == state) { - ret = 0; - goto exit; - } - - switch (state) { - case ARCHE_PLATFORM_STATE_TIME_SYNC: - if (arche_pdata->state != ARCHE_PLATFORM_STATE_ACTIVE) { - ret = -EINVAL; - goto exit; - } - if (arche_pdata->wake_detect_state != WD_STATE_IDLE) { - dev_err(arche_pdata->dev, - "driver busy with wake/detect line ops\n"); - goto exit; - } - device_for_each_child(arche_pdata->dev, NULL, - arche_apb_bootret_assert); - arche_pdata->wake_detect_state = WD_STATE_TIMESYNC; - break; - case ARCHE_PLATFORM_STATE_ACTIVE: - if (arche_pdata->state != ARCHE_PLATFORM_STATE_TIME_SYNC) { - ret = -EINVAL; - goto exit; - } - device_for_each_child(arche_pdata->dev, NULL, - arche_apb_bootret_deassert); - arche_pdata->wake_detect_state = WD_STATE_IDLE; - break; - case ARCHE_PLATFORM_STATE_OFF: - case ARCHE_PLATFORM_STATE_STANDBY: - case ARCHE_PLATFORM_STATE_FW_FLASHING: - dev_err(arche_pdata->dev, "busy, request to retry later\n"); - goto exit; - default: - ret = -EINVAL; - dev_err(arche_pdata->dev, - "invalid state transition request\n"); - goto exit; - } - arche_pdata->timesync_svc_pdata = timesync_svc_pdata; - arche_platform_set_state(arche_pdata, state); - if (state == ARCHE_PLATFORM_STATE_ACTIVE) - wake_up(&arche_pdata->wq); - - ret = 0; -exit: - spin_unlock_irqrestore(&arche_pdata->wake_lock, flags); - mutex_unlock(&arche_pdata->platform_state_mutex); - put_device(&pdev->dev); - of_node_put(np); - return ret; -} -EXPORT_SYMBOL_GPL(arche_platform_change_state); - /* Requires arche_pdata->wake_lock is held by calling context */ static void arche_platform_set_wake_detect_state( struct arche_platform_drvdata *arche_pdata, @@ -275,11 +154,6 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
spin_lock_irqsave(&arche_pdata->wake_lock, flags);
- if (arche_pdata->wake_detect_state == WD_STATE_TIMESYNC) { - gb_timesync_irq(arche_pdata->timesync_svc_pdata); - goto exit; - } - if (gpio_get_value(arche_pdata->wake_detect_gpio)) { /* wake/detect rising */
@@ -323,7 +197,6 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid) } }
-exit: spin_unlock_irqrestore(&arche_pdata->wake_lock, flags);
return IRQ_HANDLED; @@ -436,17 +309,7 @@ static ssize_t state_store(struct device *dev, struct arche_platform_drvdata *arche_pdata = platform_get_drvdata(pdev); int ret = 0;
-retry: mutex_lock(&arche_pdata->platform_state_mutex); - if (arche_pdata->state == ARCHE_PLATFORM_STATE_TIME_SYNC) { - mutex_unlock(&arche_pdata->platform_state_mutex); - ret = wait_event_interruptible( - arche_pdata->wq, - arche_pdata->state != ARCHE_PLATFORM_STATE_TIME_SYNC); - if (ret) - return ret; - goto retry; - }
if (sysfs_streq(buf, "off")) { if (arche_pdata->state == ARCHE_PLATFORM_STATE_OFF) @@ -517,8 +380,6 @@ static ssize_t state_show(struct device *dev, return sprintf(buf, "standby\n"); case ARCHE_PLATFORM_STATE_FW_FLASHING: return sprintf(buf, "fw_flashing\n"); - case ARCHE_PLATFORM_STATE_TIME_SYNC: - return sprintf(buf, "time_sync\n"); default: return sprintf(buf, "unknown state\n"); } @@ -665,7 +526,6 @@ static int arche_platform_probe(struct platform_device *pdev)
spin_lock_init(&arche_pdata->wake_lock); mutex_init(&arche_pdata->platform_state_mutex); - init_waitqueue_head(&arche_pdata->wq); arche_pdata->wake_detect_irq = gpio_to_irq(arche_pdata->wake_detect_gpio);
@@ -701,9 +561,6 @@ static int arche_platform_probe(struct platform_device *pdev) goto err_device_remove; }
- /* Register callback pointer */ - arche_platform_change_state_cb = arche_platform_change_state; - /* Explicitly power off if requested */ if (!of_property_read_bool(pdev->dev.of_node, "arche,init-off")) { mutex_lock(&arche_pdata->platform_state_mutex); diff --git a/drivers/staging/greybus/arche_platform.h b/drivers/staging/greybus/arche_platform.h index c0591df9b9d6..bcffc69d0960 100644 --- a/drivers/staging/greybus/arche_platform.h +++ b/drivers/staging/greybus/arche_platform.h @@ -15,14 +15,8 @@ enum arche_platform_state { ARCHE_PLATFORM_STATE_ACTIVE, ARCHE_PLATFORM_STATE_STANDBY, ARCHE_PLATFORM_STATE_FW_FLASHING, - ARCHE_PLATFORM_STATE_TIME_SYNC, };
-int arche_platform_change_state(enum arche_platform_state state, - struct gb_timesync_svc *pdata); - -extern int (*arche_platform_change_state_cb)(enum arche_platform_state state, - struct gb_timesync_svc *pdata); int __init arche_apb_init(void); void __exit arche_apb_exit(void);
@@ -31,7 +25,5 @@ int apb_ctrl_coldboot(struct device *dev); int apb_ctrl_fw_flashing(struct device *dev); int apb_ctrl_standby_boot(struct device *dev); void apb_ctrl_poweroff(struct device *dev); -void apb_bootret_assert(struct device *dev); -void apb_bootret_deassert(struct device *dev);
#endif /* __ARCHE_PLATFORM_H */
On 15/05/17 15:26, Johan Hovold wrote:
Remove the remaining timesync bits that were left in the arche platform driver and which prevented the driver from being compiled.
Fixes: bdfb95c4baab ("staging: greybus: remove timesync protocol support") Signed-off-by: Johan Hovold johan@kernel.org
I thought the plan was to drop arche* too ?
Why not just do that now ?
--- bod
On Mon, May 15, 2017 at 04:17:55PM +0100, Bryan O'Donoghue wrote:
On 15/05/17 15:26, Johan Hovold wrote:
Remove the remaining timesync bits that were left in the arche platform driver and which prevented the driver from being compiled.
Fixes: bdfb95c4baab ("staging: greybus: remove timesync protocol support") Signed-off-by: Johan Hovold johan@kernel.org
I thought the plan was to drop arche* too ?
Well it's not gonna live on in its current form at least.
Why not just do that now ?
Since it's still needed to boot an Ara phone.
Johan
On 15/05/17 17:10, Johan Hovold wrote:
On Mon, May 15, 2017 at 04:17:55PM +0100, Bryan O'Donoghue wrote:
On 15/05/17 15:26, Johan Hovold wrote:
Remove the remaining timesync bits that were left in the arche platform driver and which prevented the driver from being compiled.
Fixes: bdfb95c4baab ("staging: greybus: remove timesync protocol support") Signed-off-by: Johan Hovold johan@kernel.org
I thought the plan was to drop arche* too ?
Well it's not gonna live on in its current form at least.
Why not just do that now ?
Since it's still needed to boot an Ara phone.
Funny I thought the plan was to nuke it to get greybus out of staging.
On Mon, May 15, 2017 at 10:22:15PM +0100, Bryan O'Donoghue wrote:
On 15/05/17 17:10, Johan Hovold wrote:
On Mon, May 15, 2017 at 04:17:55PM +0100, Bryan O'Donoghue wrote:
On 15/05/17 15:26, Johan Hovold wrote:
Remove the remaining timesync bits that were left in the arche platform driver and which prevented the driver from being compiled.
Fixes: bdfb95c4baab ("staging: greybus: remove timesync protocol support") Signed-off-by: Johan Hovold johan@kernel.org
I thought the plan was to drop arche* too ?
Well it's not gonna live on in its current form at least.
Why not just do that now ?
Since it's still needed to boot an Ara phone.
Funny I thought the plan was to nuke it to get greybus out of staging.
It's going away in it's current form, yes. But the important part was getting rid of the dependencies in core by removing timesync. Now the platform driver no longer blocks moving core out of staging even if we keep that driver around (in staging) a while longer.
Johan
Add Arche platform-driver config option and allow the driver to be compile tested also without the usb3613 driver.
Signed-off-by: Johan Hovold johan@kernel.org --- drivers/staging/greybus/Kconfig | 10 ++++++++++ drivers/staging/greybus/Makefile | 2 +- drivers/staging/greybus/arche-platform.c | 7 +++++++ 3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig index 50de2d72dde0..cc59cf93ab9e 100644 --- a/drivers/staging/greybus/Kconfig +++ b/drivers/staging/greybus/Kconfig @@ -216,4 +216,14 @@ config GREYBUS_USB will be called gb-usb.ko
endif # GREYBUS_BRIDGED_PHY + +config GREYBUS_ARCHE + tristate "Greybus Arche Platform driver" + depends on CONFIG_USB_CONFIG_USB_HSIC_USB3613 || COMPILE_TEST + ---help--- + Select this option if you have an Arche device. + + To compile this code as a module, chose M here: the module + will be called gb-arche.ko + endif # GREYBUS diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile index b26b9a35bdd5..23e1cb7bff8e 100644 --- a/drivers/staging/greybus/Makefile +++ b/drivers/staging/greybus/Makefile @@ -91,4 +91,4 @@ obj-$(CONFIG_GREYBUS_USB) += gb-usb.o # Greybus Platform driver gb-arche-y := arche-platform.o arche-apb-ctrl.o
-obj-$(CONFIG_USB_HSIC_USB3613) += gb-arche.o +obj-$(CONFIG_GREYBUS_ARCHE) += gb-arche.o diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c index 9e644bfe2ae0..5bce5e039596 100644 --- a/drivers/staging/greybus/arche-platform.c +++ b/drivers/staging/greybus/arche-platform.c @@ -24,7 +24,14 @@ #include "arche_platform.h" #include "greybus.h"
+#if IS_ENABLED(CONFIG_USB_HSIC_USB3613) #include <linux/usb/usb3613.h> +#else +static inline int usb3613_hub_mode_ctrl(bool unused) +{ + return 0; +} +#endif
#define WD_COLDBOOT_PULSE_WIDTH_MS 30
On 05/15/17 07:26, Johan Hovold wrote:
Add Arche platform-driver config option and allow the driver to be compile tested also without the usb3613 driver.
Signed-off-by: Johan Hovold johan@kernel.org
drivers/staging/greybus/Kconfig | 10 ++++++++++ drivers/staging/greybus/Makefile | 2 +- drivers/staging/greybus/arche-platform.c | 7 +++++++ 3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig index 50de2d72dde0..cc59cf93ab9e 100644 --- a/drivers/staging/greybus/Kconfig +++ b/drivers/staging/greybus/Kconfig @@ -216,4 +216,14 @@ config GREYBUS_USB will be called gb-usb.ko endif # GREYBUS_BRIDGED_PHY
+config GREYBUS_ARCHE
- tristate "Greybus Arche Platform driver"
- depends on CONFIG_USB_CONFIG_USB_HSIC_USB3613 || COMPILE_TEST
depends on USB_HSIC_3613 || COMPILE_TEST ??
- ---help---
Select this option if you have an Arche device.
To compile this code as a module, chose M here: the module
will be called gb-arche.ko
endif # GREYBUS
On Mon, May 15, 2017 at 09:49:51AM -0700, Randy Dunlap wrote:
On 05/15/17 07:26, Johan Hovold wrote:
Add Arche platform-driver config option and allow the driver to be compile tested also without the usb3613 driver.
Signed-off-by: Johan Hovold johan@kernel.org
drivers/staging/greybus/Kconfig | 10 ++++++++++ drivers/staging/greybus/Makefile | 2 +- drivers/staging/greybus/arche-platform.c | 7 +++++++ 3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig index 50de2d72dde0..cc59cf93ab9e 100644 --- a/drivers/staging/greybus/Kconfig +++ b/drivers/staging/greybus/Kconfig @@ -216,4 +216,14 @@ config GREYBUS_USB will be called gb-usb.ko endif # GREYBUS_BRIDGED_PHY
+config GREYBUS_ARCHE
- tristate "Greybus Arche Platform driver"
- depends on CONFIG_USB_CONFIG_USB_HSIC_USB3613 || COMPILE_TEST
depends on USB_HSIC_3613 || COMPILE_TEST ??
Most certainly, not sure what happened here. I'll respin.
Thanks, Johan